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

Fix WRF-Chem reader to work with latest xarray #95

Merged
merged 2 commits into from
Dec 17, 2022

Conversation

rschwant
Copy link
Contributor

Update WRF-Chem reader to be compatible with latest version of xarray.

This fixes this bug here where this reader no longer works properly with the latest versions of xarray:
NOAA-CSL/MELODIES-MONET#149

@rschwant rschwant changed the title Develop Fix WRF-Chem reader to work with latest xarray Dec 14, 2022
@rschwant
Copy link
Contributor Author

@zmoon, can you check this out? This should fix the problem with the WRF-Chem reader in MELODIES MONET not working with the latest version of xarray. I tested and the fix works fine for older and newer versions of xarray and is a better way of doing this. Not sure why GitHub is listing the commits here for me updating my forked develop version, but I've only changed one line in one file as listed under files changed. Let me know if I need to to resubmit this pull request in some way to get rid of these extra commits. Thanks!

@zmoon
Copy link
Member

zmoon commented Dec 15, 2022

@rschwant this looks fine, thanks. I might clean up the history here if you don't mind.

Also, do you want to change it here as well?

dset = dset.reset_index(
["x", "y", "z", "z_i"], drop=True

@rschwant
Copy link
Contributor Author

Yes, feel free to clean up the history. Also good catch let's fix this in the _rrfs_cmaq reader too. I can submit this fix tomorrow or if you want to go ahead and do it, feel free.

@zmoon
Copy link
Member

zmoon commented Dec 15, 2022

The docs build failure is in the link check, for the KZ filter paper link, I'll address that separately.

@zmoon zmoon merged commit f7e73d8 into noaa-oar-arl:develop Dec 17, 2022
@rschwant
Copy link
Contributor Author

Hi, @zmoon, so I just tested this and we do not want to change the
dset = dset.reset_index(
["x", "y", "z", "z_i"], drop=True

in the RRFS-CMAQ reader. Is it easy just to change this back?

The RRFS-CMAQ output has both index and coordinate terms, which seems to mean you have to use reset_index and it will update both the index and coordinate terms.

The WRF-chem output has only coordinate terms, which appears that you have to only use the reset_coords now with the update to xarray and it will just update the coordinate terms. So the old reader for RRFS-CMAQ works fine with the latest Xarray, but the update we made here actually breaks the code.

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.

None yet

2 participants