noaa-oar-arl / monetio

The Model and ObservatioN Evaluation Tool I/O package
https://monetio.readthedocs.io
MIT License
17 stars 20 forks source link

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

Closed rschwant closed 1 year ago

rschwant commented 1 year ago

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: https://github.com/NOAA-CSL/MELODIES-MONET/issues/149

rschwant commented 1 year ago

@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 commented 1 year ago

@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? https://github.com/noaa-oar-arl/monetio/blob/683e847844515a2c19c84cfbcf41ec10fba058fb/monetio/models/_rrfs_cmaq_mm.py#L203-L204

rschwant commented 1 year ago

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 commented 1 year ago

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

rschwant commented 1 year ago

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.