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

Load JSON using importlib.resources #2351

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

efokschaner
Copy link

@efokschaner efokschaner commented Feb 4, 2022

This is a updated fix for #2308 and supersedes #2309

It uses importlib.resources which means dropping support for python 3.6 and below.

I tested using the following commands:

> python.exe .\setup.py sdist
> python.exe -m pip install .\dist\safety-db-2021.7.17.tar.gz
> python.exe
>>> import safety_db
>>> dir(safety_db)
['INSECURE', 'INSECURE_FULL', '__all__', '__author__', '__builtins__', '__cached__', '__copyright__', '__doc__', '__file__', '__license__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '__title__', '__version__', 'json', 'read_text']
>>> len(safety_db.INSECURE)
1787

I also confirmed the un-packaged dir still works as well:

>>> import data
>>> dir(data)
['INSECURE', 'INSECURE_FULL', '__all__', '__author__', '__builtins__', '__cached__', '__copyright__', '__doc__', '__file__', '__license__', '__loader__', '__name__', '__package__', '__path__', '__spec__', '__title__', '__version__', 'json', 'read_text']

This is a updated fix for pyupio#2308
and supersedes pyupio#2309

It uses importlib.resources which means dropping
support for python 3.6 and below
@bollwyvl
Copy link

bollwyvl commented Feb 4, 2022

Sure, there's that, as well as pkgutil.get_data which has a longer history of being in stdlib, and might not require the 3.7 change.

@efokschaner
Copy link
Author

@yeisonvargasf would you happen to be the right person with whom to discuss how to get this fix merged? I'd appreciate your input.

@yeisonvargasf yeisonvargasf self-requested a review March 23, 2022 01:21
@yeisonvargasf
Copy link
Member

Hi @efokschaner! Thanks for this PR, in PyUp we decided to support 3.6+ in our tools. This change requires 3.7, I would merge it as soon as possible if this removes the 3.7 requirement.

Thank you again for your work!

@efokschaner
Copy link
Author

Thanks @yeisonvargasf , I'll take a look at the approach that supports 3.6 then!

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.

3 participants