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

Pydantic for Aeroval setupclasses.py #1017

Merged
merged 62 commits into from
Apr 3, 2024
Merged

Conversation

lewisblake
Copy link
Member

Playing with pydantic to see what structure in pyaerocom/aeroval/setupclasses.py can be simplified / flattened.

@lewisblake lewisblake added style user-friendliness Updates that improve ease of use labels Feb 29, 2024
@lewisblake
Copy link
Member Author

Designed to close #1031

Copy link

codecov bot commented Mar 30, 2024

Codecov Report

Attention: Patch coverage is 95.23810% with 10 lines in your changes missing coverage. Please review.

Project coverage is 79.15%. Comparing base (5ff89df) to head (3d1e1c4).
Report is 1308 commits behind head on main-dev.

Files with missing lines Patch % Lines
pyaerocom/aeroval/setupclasses.py 94.35% 10 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##           main-dev    #1017      +/-   ##
============================================
- Coverage     79.27%   79.15%   -0.12%     
============================================
  Files           109      109              
  Lines         19023    19080      +57     
============================================
+ Hits          15080    15103      +23     
- Misses         3943     3977      +34     
Flag Coverage Δ
unittests 79.15% <95.23%> (-0.12%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@lewisblake lewisblake marked this pull request as ready for review April 2, 2024 09:19
@lewisblake
Copy link
Member Author

lewisblake commented Apr 2, 2024

Marking this as ready for review but I would really appreciate it if I could get some feedback on some of the implementation details. Starting lines 308 and 336 in setupclasses.py there are some comments which explain in more detail what I would like feedback on. Also happy to go through this in person @avaldebe after you have chance to look at it if you have any suggestions.

Copy link
Collaborator

@avaldebe avaldebe left a comment

Choose a reason for hiding this comment

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

mostly minor changes. The classes still mix data and behavior, but changing that is out of the scope of this PR.

pyaerocom/aeroval/setupclasses.py Show resolved Hide resolved
pyaerocom/aeroval/setupclasses.py Show resolved Hide resolved
pyaerocom/aeroval/setupclasses.py Outdated Show resolved Hide resolved
pyaerocom/aeroval/setupclasses.py Outdated Show resolved Hide resolved
pyaerocom/aeroval/setupclasses.py Outdated Show resolved Hide resolved
pyaerocom/aeroval/setupclasses.py Show resolved Hide resolved
pyaerocom/aeroval/setupclasses.py Outdated Show resolved Hide resolved
pyaerocom/aeroval/setupclasses.py Outdated Show resolved Hide resolved
pyaerocom/aeroval/setupclasses.py Outdated Show resolved Hide resolved
tests/aeroval/test_aeroval_HIGHLEV.py Show resolved Hide resolved
@lewisblake lewisblake requested a review from avaldebe April 3, 2024 08:41
Copy link
Collaborator

@avaldebe avaldebe left a comment

Choose a reason for hiding this comment

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

please make sure that typing_extensions is an explicit dependency.
Accordig to the typing_extensions changelog any version after 4.0.1 should work.

On pyproject.toml you'll need to add the following line

dependencies = [
[...]
    # python < 3.11
    'tomli>=2.0.1; python_version < "3.11"',
    'importlib-resources>=5.10; python_version < "3.11"',    
+   'typing-extensions>=4.0.1; python_version < "3.11"',

The equivalent line on pyaerocom_env.yml would be

dependencies:
[...]
## python < 3.11
  - tomli >=2.0.1
  - importlib-resources >=5.10
+ - typing-extensions >=4.0.1

pyaerocom/aeroval/setupclasses.py Outdated Show resolved Hide resolved
@lewisblake lewisblake requested a review from avaldebe April 3, 2024 11:46
Copy link
Collaborator

@avaldebe avaldebe left a comment

Choose a reason for hiding this comment

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

@avaldebe avaldebe merged commit a7b05c2 into main-dev Apr 3, 2024
8 checks passed
@avaldebe avaldebe deleted the pydantic-setupclasses branch April 3, 2024 12:43
@lewisblake lewisblake restored the pydantic-setupclasses branch April 3, 2024 14:12
@heikoklein heikoklein added this to the m2024-04 milestone May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
style user-friendliness Updates that improve ease of use
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants