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

Move all type-hint only imports behind if TYPE_CHECKING #354

Merged
merged 2 commits into from
May 23, 2023
Merged

Move all type-hint only imports behind if TYPE_CHECKING #354

merged 2 commits into from
May 23, 2023

Conversation

janosh
Copy link
Member

@janosh janosh commented May 23, 2023

Summary

1ab9e8f ruff enable flake8-type-checking (TCH)
2f54128 move all type-hint only imports behind if TYPE_CHECKING

Related PR: materialsproject/pymatgen#2992

4 main motivations for this change:

  1. reduces chance of circular imports
  2. can help contributors run tests without installing extra deps like pymatgen.analysis.defects if they were previously in a test's import stack but only used for type hints
  3. helps with readability by clearly separating type-only imports
  4. can decrease startup time since TYPE_CHECKING=False at run time means some modules won't have to be imported. somewhat negated by the annoyingly long import time of the std lib typing module itself but that has been steadily coming down with newer Python versions.

@janosh janosh added the imports label May 23, 2023
@codecov
Copy link

codecov bot commented May 23, 2023

Codecov Report

Merging #354 (2f54128) into main (7bef4b0) will decrease coverage by 1.33%.
The diff coverage is 19.73%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #354      +/-   ##
==========================================
- Coverage   66.07%   64.74%   -1.33%     
==========================================
  Files          74       74              
  Lines        7101     7157      +56     
  Branches      904      938      +34     
==========================================
- Hits         4692     4634      -58     
- Misses       2140     2224      +84     
- Partials      269      299      +30     
Impacted Files Coverage Δ
src/atomate2/amset/files.py 0.00% <0.00%> (ø)
src/atomate2/vasp/builders/elastic.py 0.00% <0.00%> (ø)
src/atomate2/vasp/flows/amset.py 0.00% <0.00%> (ø)
src/atomate2/vasp/jobs/amset.py 0.00% <0.00%> (ø)
src/atomate2/common/jobs/defect.py 83.48% <14.28%> (-5.41%) ⬇️
src/atomate2/common/flows/defect.py 82.08% <16.66%> (-7.15%) ⬇️
src/atomate2/vasp/flows/elastic.py 82.05% <16.66%> (-12.55%) ⬇️
src/atomate2/vasp/flows/phonons.py 88.88% <16.66%> (-5.43%) ⬇️
src/atomate2/vasp/jobs/elastic.py 78.31% <16.66%> (-5.64%) ⬇️
src/atomate2/vasp/jobs/phonons.py 80.89% <16.66%> (-5.31%) ⬇️
... and 24 more

@utf
Copy link
Member

utf commented May 23, 2023

Great, thank you!

@utf utf merged commit 4602d21 into main May 23, 2023
@janosh janosh deleted the ruff-tch branch June 2, 2023 18:27
@utf utf added enhancement Improvements to existing features house-keeping Formatting and code quality tweaks and removed enhancement Improvements to existing features labels Sep 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
house-keeping Formatting and code quality tweaks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants