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

add aerosol related updates #161

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

bbakernoaa
Copy link

This closes #160

Main updates were in the template.py and _grib2io.py files. More improvement could be completed later but it does produce a grib2 message now with unique shortNames and fullNames for each variable now.

image

Additionally the xarray backend works as well.

image

image

@bbakernoaa
Copy link
Author

@zmoon

@bbakernoaa
Copy link
Author

This is still in a draft mode. Please don't merge yet

@bbakernoaa
Copy link
Author

This should be ready now.

@bbakernoaa
Copy link
Author

ok should be good to go. More work is needed but this at least adds the capability for GEFS-Aerosols and RRFS to properly read the variables and assign unique variable names to each aerosol species.

Copy link

@zmoon zmoon left a comment

Choose a reason for hiding this comment

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

Just some cleanup comments. Also maybe the aerosol-specific get short/full name sections could be extracted to their own function/method.

log Outdated Show resolved Hide resolved
src/grib2io/_grib2io.py Outdated Show resolved Hide resolved
src/grib2io/xarray_backend.py Outdated Show resolved Hide resolved
src/grib2io/templates.py Outdated Show resolved Hide resolved
src/grib2io/templates.py Show resolved Hide resolved
src/grib2io/templates.py Outdated Show resolved Hide resolved
src/grib2io/xarray_backend.py Outdated Show resolved Hide resolved
src/grib2io/xarray_backend.py Outdated Show resolved Hide resolved
src/grib2io/xarray_backend.py Outdated Show resolved Hide resolved
@EricEngle-NOAA
Copy link
Collaborator

How is this coming along? Do you think it is ready to merge?

@bbakernoaa
Copy link
Author

Yes I believe it is ready to go. This actually led us to find an error in the way they were assigning a grib variable in the RRFS prototype too. They were assigning the aerosol type as "Missing" for some of their species instead of total or 62000

src/grib2io/tables/section4.py Show resolved Hide resolved
src/grib2io/_grib2io.py Show resolved Hide resolved
src/grib2io/templates.py Show resolved Hide resolved
log Outdated Show resolved Hide resolved
@EricEngle-NOAA
Copy link
Collaborator

Forgot to post these questions from my review a few days ago. Apologies.

@bbakernoaa
Copy link
Author

@EricEngle-NOAA Anything you want me to do to get this merged?

@EricEngle-NOAA
Copy link
Collaborator

@bbakernoaa can you create a test for this? See the tests/ dir at the repo root. We are using pytest. Also feel free to add a GEFS chem file to tests/data/, but thin the grid to 1.0 deg to keep a smaller file size.

@EricEngle-NOAA
Copy link
Collaborator

I think a test to make sure the shortName and fullName are properly constructed is good enough. That is all that this PR is for so no need to test data unpacking etc.

@bbakernoaa
Copy link
Author

Ok will do. I added the 2d data file here but may also add one for the 3d data

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.

Not able to read the GEFS-Aerosol grib2 data
3 participants