xMIP icon indicating copy to clipboard operation
xMIP copied to clipboard

Add `longitude_bnds` and `latitude_bnds` to `cmip_renaming_dict`

Open JoranAngevaare opened this issue 1 year ago • 7 comments

Add longitude_bnds and latitude_bnds to cmip_renaming_dict, this allows renaming for example renaming bounds for 'ScenarioMIP.UA.MCM-UA-1-0.ssp585.r1i1p1f2.Amon.gn.none.tas' .

  • [x] Tests added
  • [x] Passes pre-commit run --all-files
  • [x] Passes pytest
  • [x] User visible changes (including notable bug fixes) are documented in whats-new.rst

JoranAngevaare avatar Jun 01 '23 13:06 JoranAngevaare

Thank you very much for adding these @JoranAngevaare. I think the testing failure is unrelated (looks like #301). Let me take a crack at that and see if that resolves the CI failures here. Sorry for the delay.

jbusecke avatar Jun 07 '23 19:06 jbusecke

Ok I am fairly confident that the above issue is fixed by https://github.com/jbusecke/xarrayutils/pull/162, since the upstream-dev tests are now passing. Once v2.0.0 is published on conda-forge I expect the other tests to pass.

jbusecke avatar Jun 09 '23 18:06 jbusecke

Ok I got it all sorted. Thanks for your patience @JoranAngevaare. Would you mind adding an entry to the docs/whats-new.rst? Once that is done, ill merge this right away!

jbusecke avatar Jun 09 '23 20:06 jbusecke

@jbusecke thanks a lot for all the fixes! I see that numpy 1.24.0 drop of deprecated np.float caused some issues, the np.zeros(.., float) will be interpreted as np.zeros(.., np.float64)'s which would also have solved that particular problem.

I added a trivial test eff95dc, however, that lead me to notice that bnds is renamed to bnds, I'm not entirely sure if that is a desirable feature. If so, I'll very quickly revert a6a5835.

JoranAngevaare avatar Jun 12 '23 07:06 JoranAngevaare

that lead me to notice that bnds is renamed to bnds

I am not sure I follow this. I do intent to rename the bounds dimension similarly to others, but I might be missing something in your comment.

jbusecke avatar Jun 20 '23 15:06 jbusecke

Hi @JoranAngevaare again sorry for the delay (the amount of times I have typed this today 😆). I am not quite following myself from the past TBH. I now completely agree that renaming "bnds" to "bnds" is nonsensical. Sorry for the back and forth. Could you restore that change? Ill get this moving right after the tests pass.

jbusecke avatar Jan 25 '24 20:01 jbusecke

I would also like to add you to the CITATION.cff for the contributions you made here.

jbusecke avatar Jan 25 '24 21:01 jbusecke

@JoranAngevaare finally getting around to merging this. Again sorry for the delay. I have opened #358 to track the addition to the CITATION file.

jbusecke avatar Jun 25 '24 03:06 jbusecke

hi @jbusecke sorry for the delay on my side - I missed these previous comments.

JoranAngevaare avatar Jun 25 '24 14:06 JoranAngevaare