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

Modernize all Python 3.8 annotations #1815

Merged
merged 5 commits into from Oct 15, 2023
Merged

Modernize all Python 3.8 annotations #1815

merged 5 commits into from Oct 15, 2023

Conversation

ml-evs
Copy link
Member

@ml-evs ml-evs commented Oct 15, 2023

This PR is complementary to #1745. I will first investigate how easy it is to merge the two...

  • Add pyupgrade --py39-plus as a pre-commit hook and run it
  • Use 3.9 as base version of CI/pre-commit to avoid any compatibility issues between tools

.pre-commit-config.yaml Outdated Show resolved Hide resolved
optimade/adapters/base.py Show resolved Hide resolved
optimade/adapters/structures/adapter.py Show resolved Hide resolved
optimade/adapters/structures/ase.py Show resolved Hide resolved
optimade/adapters/structures/cif.py Show resolved Hide resolved
optimade/server/entry_collections/elasticsearch.py Outdated Show resolved Hide resolved
optimade/server/exception_handlers.py Show resolved Hide resolved
optimade/server/schemas.py Show resolved Hide resolved
optimade/validator/utils.py Show resolved Hide resolved
tests/models/test_utils.py Show resolved Hide resolved
ml-evs and others added 2 commits October 15, 2023 14:26
Co-authored-by: Casper Welzel Andersen <43357585+CasperWA@users.noreply.github.com>
Co-authored-by: Casper Welzel Andersen <43357585+CasperWA@users.noreply.github.com>
@ml-evs
Copy link
Member Author

ml-evs commented Oct 15, 2023

I have to dash, @CasperWA once the tests pass here I will merge and then force update the big migration PR... I have kept a backup at ml-evs/backup-pydantic-v2-orig that we can always go back to, but otherwise I will review the rebased version next week. Thanks again for your help!

@codecov
Copy link

codecov bot commented Oct 15, 2023

Codecov Report

Merging #1815 (ef611f8) into master (cf45488) will decrease coverage by 0.10%.
The diff coverage is 99.07%.

@@            Coverage Diff             @@
##           master    #1815      +/-   ##
==========================================
- Coverage   90.77%   90.68%   -0.10%     
==========================================
  Files          74       74              
  Lines        4629     4616      -13     
==========================================
- Hits         4202     4186      -16     
- Misses        427      430       +3     
Flag Coverage Δ
project 90.68% <99.07%> (-0.10%) ⬇️
validator 90.57% <99.07%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
optimade/adapters/base.py 96.96% <100.00%> (ø)
optimade/adapters/references/adapter.py 100.00% <100.00%> (ø)
optimade/adapters/structures/adapter.py 100.00% <100.00%> (ø)
optimade/adapters/structures/aiida.py 100.00% <100.00%> (ø)
optimade/adapters/structures/ase.py 98.61% <100.00%> (-0.02%) ⬇️
optimade/adapters/structures/cif.py 84.09% <100.00%> (-0.36%) ⬇️
optimade/adapters/structures/proteindatabank.py 89.28% <100.00%> (-0.13%) ⬇️
optimade/adapters/structures/pymatgen.py 98.50% <100.00%> (ø)
optimade/adapters/structures/utils.py 80.88% <100.00%> (+0.14%) ⬆️
optimade/client/client.py 81.01% <100.00%> (-0.92%) ⬇️
... and 34 more

@ml-evs ml-evs merged commit 8510ffa into master Oct 15, 2023
11 checks passed
@JPBergsma
Copy link
Contributor

I think there are still many more locations where we still import the type definitions.
for example: entry_collections/mongo.py
optimade/client/client.py

Should these also be changed?

@ml-evs
Copy link
Member Author

ml-evs commented Oct 16, 2023

I think there are still many more locations where we still import the type definitions. for example: entry_collections/mongo.py optimade/client/client.py

Should these also be changed?

Do you mean Union, Optional etc? These are still required for Python 3.9 (but not for 3.10). I think the pyupgrade hook dealt with everything we can get away with for now.

@JPBergsma
Copy link
Contributor

Looks like there was an issue with caching. After a reboot, I no longer get any hits when I search for "Tuple".

ml-evs added a commit that referenced this pull request Nov 6, 2023
* Use Python 3.9 as the 'base' CI version for linting

* Update pre-commit hooks

* Run `pyupgrade --py39-plus` to upgrade legacy annotations

* Add `--exit-non-zero-on-fix` for ruff

Co-authored-by: Casper Welzel Andersen <43357585+CasperWA@users.noreply.github.com>

* Use f-string over format

Co-authored-by: Casper Welzel Andersen <43357585+CasperWA@users.noreply.github.com>

---------

Co-authored-by: Casper Welzel Andersen <43357585+CasperWA@users.noreply.github.com>
ml-evs added a commit that referenced this pull request Nov 6, 2023
* Use Python 3.9 as the 'base' CI version for linting

* Update pre-commit hooks

* Run `pyupgrade --py39-plus` to upgrade legacy annotations

* Add `--exit-non-zero-on-fix` for ruff

Co-authored-by: Casper Welzel Andersen <43357585+CasperWA@users.noreply.github.com>

* Use f-string over format

Co-authored-by: Casper Welzel Andersen <43357585+CasperWA@users.noreply.github.com>

---------

Co-authored-by: Casper Welzel Andersen <43357585+CasperWA@users.noreply.github.com>
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.

None yet

3 participants