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

46 implement actris ebas reader #48

Merged
merged 114 commits into from
Apr 2, 2025

Conversation

jgriesfeller
Copy link
Member

1st iteration of an ACTRIS-EBAS reader (1st because neither the API nor the netcdf file's format is stable yet).

Done due to the need to test and use what NILU does to find errors on their side

closes #46

@jgriesfeller jgriesfeller added this to the m2024-11 milestone Aug 29, 2024
@jgriesfeller jgriesfeller self-assigned this Aug 29, 2024
@jgriesfeller jgriesfeller linked an issue Aug 29, 2024 that may be closed by this pull request
@jgriesfeller jgriesfeller marked this pull request as ready for review April 2, 2025 09:18
Copy link
Collaborator

@magnusuMET magnusuMET left a comment

Choose a reason for hiding this comment

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

The implementation is rather complicated to understand since it downloads data on use instead of acting on pre-downloaded files. I wish the download part could be separated from making the data accessible for pyaerocom

def metadata(self):
return self._metadata

def _read(
Copy link
Collaborator

Choose a reason for hiding this comment

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

This function is eager and does all the reading of all variables at once, where as I think we would like to have the reading done when calling data(var_to_read), for minimising the amount of data kept per iteration. Just a note as refactoring at this point might be a lot of work.

Copy link
Member Author

@jgriesfeller jgriesfeller Apr 2, 2025

Choose a reason for hiding this comment

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

The problem is that in pyaerocom there's a check against supported variables.

https://github.com/metno/pyaerocom/blob/43f6fce139d1b09facc9ed01cb96d14e287b192f/pyaerocom/io/pyaro/read_pyaro.py#L48-L52

For that info to get, pyro has to know the variable names AFTER the variable name changing reader and is calling data(var_to_read) for that. And that is the reason the data is read early on (if I understood the code correctly).
In any case, this reader needs an variable name filter anyway.

jgriesfeller and others added 8 commits April 2, 2025 15:10
@jgriesfeller
Copy link
Member Author

jgriesfeller commented Apr 2, 2025

The nature of this reader is to work online. The data can change at any time and it would too computational heavy to download all data from Nilu (for Nilu). Therefore the online reading is intended and part of the contract with ACTRIS.

Copy link
Collaborator

@magnusuMET magnusuMET left a comment

Choose a reason for hiding this comment

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

Approving to include this reader as a prototype for the release of this month

@jgriesfeller jgriesfeller merged commit b684613 into main-dev Apr 2, 2025
4 checks passed
@jgriesfeller jgriesfeller deleted the 46-implement-actris-ebas-reader branch April 2, 2025 13:57
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.

implement ACTRIS-EBAS reader
3 participants