Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pythonlib: Fix ResourceWarning in libgrass_interface_generator/ctypesgen/version.py #4288

Closed
wants to merge 4 commits into from

Conversation

arohanajit
Copy link
Contributor

This pull request addresses the recurring ResourceWarning about an unclosed file in the version.py script. The following changes have been made:

These changes should resolve the ResourceWarnings in this particular case

@github-actions github-actions bot added Python Related code is in Python libraries labels Sep 8, 2024
echoix
echoix previously requested changes Sep 8, 2024
python/libgrass_interface_generator/ctypesgen/version.py Outdated Show resolved Hide resolved
Co-authored-by: Edouard Choinière <27212526+echoix@users.noreply.github.com>
devnull = open(os.devnull, "w")
p = Popen(["git", "describe"], stdout=PIPE, stderr=devnull, **args)
with open(os.devnull, "w") as devnull:
p = Popen(["git", "describe"], stdout=PIPE, stderr=devnull, **args)

Check notice

Code scanning / Bandit

Starting a process with a partial executable path Note

Starting a process with a partial executable path
@arohanajit arohanajit changed the title Fix ResourceWarning for python/libgrass_interface_generator/ctypesgen/version.py Fix ResourceWarning for libgrass_interface_generator/ctypesgen/version.py Sep 8, 2024
Copy link
Contributor

@nilason nilason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We shouldn't waste time on non-crucial fixes here on ctypesgen, with next update from upstream, they will be null-and-void (see https://github.com/OSGeo/grass/blob/main/python/libgrass_interface_generator/README.md).

@echoix
Copy link
Member

echoix commented Sep 9, 2024

Do you know if upstream is planning to make a release real soon?

@nilason
Copy link
Contributor

nilason commented Sep 9, 2024

Do you know if upstream is planning to make a release real soon?

It looks like it's pretty much up to me unfortunately, as I'm the only active with enough access rights. So, probably not "real" soon. Still, these kind of changes are not urgent.

@echoix
Copy link
Member

echoix commented Sep 9, 2024

Would you want these changes in a PR in the upstream repo so they could be there when ready?

@arohanajit arohanajit changed the title Fix ResourceWarning for libgrass_interface_generator/ctypesgen/version.py pythonlib: Fix ResourceWarning in libgrass_interface_generator/ctypesgen/version.py Sep 11, 2024
@arohanajit
Copy link
Contributor Author

@echoix just wanted to confirm if these are still required. If not, I'll close this PR

@echoix
Copy link
Member

echoix commented Sep 12, 2024

We might as well close them. But they still exist to search again at some point. It's kinda sad, but otherwise it'll end up like one of the 130 other pending PRs

@arohanajit arohanajit closed this Sep 12, 2024
@arohanajit arohanajit deleted the pytest-ruff branch September 20, 2024 17:56
@arohanajit arohanajit restored the pytest-ruff branch September 20, 2024 17:56
@arohanajit arohanajit deleted the pytest-ruff branch September 20, 2024 19:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libraries Python Related code is in Python
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants