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

Replace pkg_resources with importlib #574

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

chrisjonesBSU
Copy link
Contributor

@chrisjonesBSU chrisjonesBSU commented Jul 13, 2024

PR Summary:

I'm trying to fix some of the deprecation errors we're getting in the mosdef packages. One of them is coming from foyer with the use of pkg_resources.

pkg_resources will be removed in python 3.13

Note: This PR is not compatible with python 3.9, so we can't merge it until we want to drop 3.9 support and add 3.13.

PR Checklist


  • Includes appropriate unit test(s)
  • Appropriate docstring(s) are added/updated
  • Code is (approximately) PEP8 compliant
  • Issue(s) raised/addressed?

@chrisjonesBSU
Copy link
Contributor Author

chrisjonesBSU commented Jul 14, 2024

It looks like the entry points methods in python 3.9 is different from python >= 3.10. So, that means we can't support both 3.9 and 3.13 whenever it's released. We can table these changes until we're ready to move on to python >= 3.10.

@chrisjonesBSU chrisjonesBSU changed the title Replace pkg_resources with importlib; use lark in .yml files instead of lark-parser Replace pkg_resources with importlib Aug 19, 2024
Copy link

codecov bot commented Aug 19, 2024

Codecov Report

Attention: Patch coverage is 44.44444% with 5 lines in your changes missing coverage. Please review.

Project coverage is 62.02%. Comparing base (2707210) to head (ec895b6).
Report is 7 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #574      +/-   ##
==========================================
- Coverage   62.04%   62.02%   -0.02%     
==========================================
  Files          16       16              
  Lines        1665     1667       +2     
==========================================
+ Hits         1033     1034       +1     
- Misses        632      633       +1     

@CalCraven
Copy link
Contributor

I don't think it's necessarily the right call here, but we could opt for using
from platform import python_version
to handle the differentiation from python 3.9-3.13 in this PR. Only if we didn't want to wait until we have to fully drop 3.9 to merge this PR.

@chrisjonesBSU
Copy link
Contributor Author

I don't think it's necessarily the right call here, but we could opt for using from platform import python_version to handle the differentiation from python 3.9-3.13 in this PR. Only if we didn't want to wait until we have to fully drop 3.9 to merge this PR.

I say we just wait. Maybe at the next dev meeting we can start talking about the plan to drop 3.9 and add 3.13, which will be officially released next month I believe.

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

Successfully merging this pull request may close these issues.

2 participants