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

Use importlib.resources instead of pkg_resources (from setuptools) #278

Merged
merged 1 commit into from
May 9, 2024

Conversation

drvdputt
Copy link
Contributor

This seems to work fine to resolve #277 . Let's see what the tests do.

@drvdputt
Copy link
Contributor Author

drvdputt commented May 7, 2024

In addition to the pull requests planned in #280 , can we pull this one in first?

@jdtsmith
Copy link
Contributor

jdtsmith commented May 9, 2024

I know very little about packaging, but happy to merge this. Can you target this to the dev branch, which I just created as a clean branch from master?

@drvdputt drvdputt changed the base branch from master to dev May 9, 2024 16:42
@drvdputt
Copy link
Contributor Author

drvdputt commented May 9, 2024

The issue here is that we were using a packaging tool (always had to pip install setuptools) to deal with the import of our resource files (the example spectra and included science packs).

This was never a problem with the automated tests, since they seem to always have setuptools in their environment. This is why we never included setuptools as a dependency. So when setting up a virtual environment with python3 -m venv venv_name, it is not always guaranteed that setuptools is present.

With these changes, we use the native importlib, so one less dependency. I think we can merge this into dev. If any thing breaks, I'm sure all the other testing during development will reveal it.

@jdtsmith jdtsmith merged commit 4740c34 into PAHFIT:dev May 9, 2024
14 of 15 checks passed
@jdtsmith
Copy link
Contributor

jdtsmith commented May 9, 2024

Great done. I'm pretty ignorant on all the flavors of setup* stuff. I can at least pip install a local directory though ;).

@drvdputt drvdputt mentioned this pull request May 9, 2024
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.

Use of pkg_resources
2 participants