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

Add some type annotations #578

Merged
merged 4 commits into from
Oct 19, 2023
Merged

Conversation

ab5424
Copy link
Contributor

@ab5424 ab5424 commented Oct 19, 2023

Summary

  • Added some type hints, mostly return types
  • Ran pyupgrade --py39-plus
  • Added pygrep-hooks rule and fixed some errors

Checklist

  • Code is in the standard Python style.
    The easiest way to handle this is to run the following in the correct sequence on
    your local machine. Start with running black on your new code. This will
    automatically reformat your code to PEP8 conventions and removes most issues. Then run
    ruff.
  • Docstrings have been added in the Numpy docstring format.
    Run ruff on your code.
  • Type annotations are highly encouraged. Run mypy to
    type check your code.
  • Tests have been added for any new functionality or bug fixes.
  • All linting and tests pass.

Copy link
Member

@janosh janosh Oct 19, 2023

Choose a reason for hiding this comment

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

Any schema files that define pydantic models cannot use future type annotations until atomate2 is 3.10+. That won't happen for at least another year.

You can revert all schema files with something like (untested!)

git checkout main -- **/schema/* **/schemas.py

@janosh
Copy link
Member

janosh commented Oct 19, 2023

Also, feel free to add yourself to contributors.md.

@JaGeo
Copy link
Member

JaGeo commented Oct 19, 2023

Also, feel free to add yourself to contributors.md.

I added @ab5424 yesterday (contributor to phonon workflow)

@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

Merging #578 (aa22dd2) into main (4e3d156) will decrease coverage by 0.31%.
Report is 4 commits behind head on main.
The diff coverage is 80.00%.

❗ Current head aa22dd2 differs from pull request most recent head c3d6a4e. Consider uploading reports for the commit c3d6a4e to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #578      +/-   ##
==========================================
- Coverage   75.62%   75.31%   -0.31%     
==========================================
  Files          80       80              
  Lines        6711     6722      +11     
  Branches      992      992              
==========================================
- Hits         5075     5063      -12     
- Misses       1330     1354      +24     
+ Partials      306      305       -1     
Files Coverage Δ
src/atomate2/amset/jobs.py 0.00% <ø> (ø)
src/atomate2/common/files.py 81.00% <ø> (ø)
src/atomate2/common/flows/defect.py 81.57% <100.00%> (ø)
src/atomate2/common/jobs/phonons.py 84.70% <ø> (ø)
src/atomate2/common/schemas/cclib.py 77.55% <100.00%> (ø)
src/atomate2/common/schemas/defects.py 87.91% <100.00%> (ø)
src/atomate2/common/schemas/elastic.py 88.65% <100.00%> (ø)
src/atomate2/common/schemas/phonons.py 97.12% <100.00%> (ø)
src/atomate2/cp2k/drones.py 53.57% <100.00%> (ø)
src/atomate2/cp2k/jobs/base.py 89.23% <100.00%> (ø)
... and 52 more

... and 1 file with indirect coverage changes

Copy link
Member

@janosh janosh left a comment

Choose a reason for hiding this comment

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

I'm surprised the tests pass! Maybe the pydantic restriction I mentioned no longer applies. Looks like the recent migration from pydantic 1 to 2 did away with that.

@janosh janosh merged commit 109d9ef into materialsproject:main Oct 19, 2023
@janosh janosh mentioned this pull request Oct 19, 2023
@janosh janosh added ux User experience pkg Package health and distribution related stuff types Type all the things qa labels Oct 19, 2023
@utf utf added the house-keeping Formatting and code quality tweaks label Oct 21, 2023
@ab5424 ab5424 deleted the annotations-auto branch October 27, 2023 12:20
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 pkg Package health and distribution related stuff types Type all the things ux User experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants