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

Get version without deprecation warnings #29

Merged
merged 3 commits into from
Jul 20, 2024

Conversation

bgyori
Copy link
Contributor

@bgyori bgyori commented Jul 19, 2024

The line from pkg_resources import DistributionNotFound, get_distribution produces deprecation warnings (see https://setuptools.pypa.io/en/latest/pkg_resources.html). This PR implements a variant for Python 3.8+ that avoids these deprecations.

Comment on lines 20 to 26
else:
from pkg_resources import DistributionNotFound, get_distribution

try:
return str(get_distribution("obonet").version)
except DistributionNotFound:
return None
Copy link
Owner

@dhimmel dhimmel Jul 19, 2024

Choose a reason for hiding this comment

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

Thanks for the contribution!

What do you think about bumping to 3.8 at

requires-python = ">=3.7"

target-version = "py37"

and then removing the from pkg_resources import DistributionNotFound, get_distribution approach altogether?

Copy link
Owner

Choose a reason for hiding this comment

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

3.7 end of life as of June 27, 2023 and pip should know to install an older release of obonet if using python 3.7

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's fine with me, I tend to always err on the side of broader compatibility unless there is a meaningful feature being blocked by it, but I think in this case, this would not be a big issue.

Copy link
Owner

@dhimmel dhimmel Jul 19, 2024

Choose a reason for hiding this comment

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

Cool. I'm happy to make the changes after merging this or you can. Let me know which you prefer!

I tend to always err on the side of broader compatibility unless there is a meaningful feature being blocked by it, but I think in this case, this would not be a big issue.

Yes compatibility is good but also making this package as easy to maintain as possible is also good.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I made the changes, feel free to make further modifications if needed.

Copy link
Owner

@dhimmel dhimmel left a comment

Choose a reason for hiding this comment

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

Thanks! I'll drop a note here once there's a release.

@dhimmel dhimmel merged commit ffd8310 into dhimmel:main Jul 20, 2024
6 of 9 checks passed
@dhimmel
Copy link
Owner

dhimmel commented Jul 20, 2024

Release 1.1.0 now on PyPI contains the merged commit.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants