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

32 add pyaro ebas reader for nilu pmf data as well as general use #35

Conversation

jgriesfeller
Copy link
Member

add EBAS reader that doesn't need the SQLite database since it's not provided by the PMF dataset.
Use as much from the pyaerocom EBAS reader as possible.

@jgriesfeller jgriesfeller added the enhancement New feature or request label Apr 10, 2024
@jgriesfeller jgriesfeller added this to the m2024-05 milestone Apr 10, 2024
@jgriesfeller jgriesfeller self-assigned this Apr 10, 2024
@jgriesfeller jgriesfeller linked an issue Apr 10, 2024 that may be closed by this pull request
@jgriesfeller
Copy link
Member Author

jgriesfeller commented Apr 10, 2024

After half a day of working on this, I realised that the approach of transforming the existing EBAS reader will not work due to too deep cross imports / improper dependency handling. Importing the EBAS reader class has even a dependency on the gridded data object.

I will therefore start from pyaro and just steal from the EBAS reader class what's really needed for PMF while trying to keep the readability of all ebas variables supported by pyaerocom. At this point I'm not sure how well that will work. The focus might become to just read the PMF data.

@dulte dulte mentioned this pull request Apr 16, 2024
@jgriesfeller jgriesfeller marked this pull request as ready for review May 6, 2024 14:30
@jgriesfeller
Copy link
Member Author

The pyaro-readers part is finished, but we are missing the pyaerocom glue to EBAS. The problem is that for EBAS data there is no 1:1 connection between the variables.
Example:
There's #mannosan#ng m-3, pm10#mannosan#ng m-3 and pm25#mannosan#ng m-3 in these files. Usually the users want the variable in a certain proiority read. How do we achieve that? Create an new reader in pyaerocom like the old way, or do we make pyaro-readers support pyaerocom variables?

@jgriesfeller jgriesfeller requested a review from thorbjoernl May 6, 2024 14:42
Copy link
Collaborator

@thorbjoernl thorbjoernl left a comment

Choose a reason for hiding this comment

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

Suggested a handful of changes.

It would also be good to add docstrings and perhaps type-hints to functions, as some of them lack clear documentation currently.

@jgriesfeller jgriesfeller requested a review from thorbjoernl May 7, 2024 14:01
Copy link
Collaborator

@thorbjoernl thorbjoernl left a comment

Choose a reason for hiding this comment

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

LGTM 🚀 🚀

Jan Jurgen Griesfeller added 2 commits May 8, 2024 10:56
…nilu-pmf-data-as-well-as-general-use

# Conflicts:
#	setup.cfg
…u-pmf-data-as-well-as-general-use' into 32-add-pyaro-ebas-reader-for-nilu-pmf-data-as-well-as-general-use

# Conflicts:
#	setup.cfg
@jgriesfeller jgriesfeller merged commit c2f973b into main-dev May 8, 2024
4 checks passed
@jgriesfeller jgriesfeller deleted the 32-add-pyaro-ebas-reader-for-nilu-pmf-data-as-well-as-general-use branch May 8, 2024 09:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Pyaro EBAS reader for NILU PMF data, as well as general use
2 participants