Skip to content

Replace pkg_resources with importlib_metadata #250

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

Merged
merged 1 commit into from
Jun 2, 2023
Merged

Replace pkg_resources with importlib_metadata #250

merged 1 commit into from
Jun 2, 2023

Conversation

jbylund
Copy link
Contributor

@jbylund jbylund commented May 25, 2023

Copying description from issue:

pkg_resources is now deprecated in favor of importlib.metadata / importlib_metadata (source). Right now it causes warnings and in the future (3.12?) I think it will stop working entirely. I saw there was a related pr but it was using importlib.metadata which apparently has different behaviors on some versions of python and caused issues for some users (ref). Maybe it would be acceptable to use importlib_metadata (the backport) either on all versions of python or only when necessary.

@apetenchea apetenchea self-requested a review May 26, 2023 13:38
from requests import Session
import importlib_metadata
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for spotting that!

I would be ready to accept this PR, but the pre-commit hook isort is failing due to the non-standard ordering of imports. Please see CONTRIBUTING.md for more details.

@jbylund
Copy link
Contributor Author

jbylund commented May 31, 2023

I'm a little worried about the use case of #244 (using without installing via just adding to path). But it seems to work for me:

In [5]: import arango

In [6]: arango.__file__
Out[6]: '/Users/joebylund/scratch/python-arango/arango/__init__.py'

In [7]: ArangoClient().version
Out[7]: '7.5.8.dev2+gfac52f3'

@jbylund jbylund requested a review from apetenchea May 31, 2023 11:23
@apetenchea
Copy link
Member

I'm a little worried about the use case of #244 (using without installing via just adding to path). But it seems to work for me:

In [5]: import arango

In [6]: arango.__file__
Out[6]: '/Users/joebylund/scratch/python-arango/arango/__init__.py'

In [7]: ArangoClient().version
Out[7]: '7.5.8.dev2+gfac52f3'

That's a valid concern, I'll have to test it out as well. Will get to it by the end of this week.

Copy link
Member

@apetenchea apetenchea left a comment

Choose a reason for hiding this comment

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

I tried it locally as well and it works for me. However, I am hesitant to merge it yet, without @alexbakharew 's input, as this can potentially affect some of our internal projects.

@apetenchea
Copy link
Member

apetenchea commented Jun 2, 2023

Alright, I talked to @alexbakharew and he confirmed it's working.
Thank you for your contribution!

@apetenchea apetenchea merged commit 4a45183 into arangodb:main Jun 2, 2023
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