EcoExtreML / zampy

Tool for downloading Land Surface Model input data
https://zampy.readthedocs.io/
Apache License 2.0
1 stars 0 forks source link

Implement era5 land #21

Closed geek-yang closed 1 year ago

geek-yang commented 1 year ago

Support ERA5 land dataset in zampy.

review-notebook-app[bot] commented 1 year ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

geek-yang commented 1 year ago

Ok...Linter fails because of another abstract class thingy, due to protocol. Apparently the latest release of mypy is quite picky about abstract class.

BSchilperoort commented 1 year ago

It would be nice to keep the type hint here though:

DATASETS: dict[str, type[dataset_protocol.Dataset]] = {

Perhaps use Union[type[dataset_protocol.Dataset], type[ECMWFDataset]]? Then we get type checking in the recipe runner, otherwise the datasets there would resolve to Any.

Although I would have thought we can just provide the protocol as type and things should work: https://mypy.readthedocs.io/en/stable/protocols.html#simple-user-defined-protocols

Perhaps ECMWFDataset needs to subclass protocol as well... see: https://mypy.readthedocs.io/en/stable/protocols.html#defining-subprotocols-and-subclassing-protocols :thinking:

sonarcloud[bot] commented 1 year ago

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 3 Code Smells

98.7% 98.7% Coverage
0.0% 0.0% Duplication