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

Move private _(global)_stock_cube from esmvacore.preprocessor._regrid to cmorizer #2087

Merged
merged 3 commits into from Mar 19, 2021

Conversation

valeriupredoi
Copy link
Contributor

Private import and on top of it all, func changed name from _stock_cube to _global_stock_cube. A bit clunky since I had to move a host of global variables but heyho, more secure now.

@valeriupredoi
Copy link
Contributor Author

can I pls get an OK for this one guys, tests are failing coz of the need for this 👍 🍺

@schlunma
Copy link
Contributor

Code looks good (I guess you just copy-pasted from ESMValCore, right?)! But it probably wouldn't hurt to test the output of the CMORizer and compare it to the old version. I will do that briefly!

@valeriupredoi
Copy link
Contributor Author

many thanks, Manu 🍺

Copy link
Contributor

@schlunma schlunma left a comment

Choose a reason for hiding this comment

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

I had to fix the CMORizer because of a

iris.exceptions.UnitConversionError: Cannot convert from unknown units. The "cube.units" attribute may be set directly.

but the output is identical to the data we have stored on mistral (I only checked the years 1961 and 1962 though to save computation time) 👍

@valeriupredoi
Copy link
Contributor Author

@schlunma you a total trooper, cheers bro! 🍺

@valeriupredoi valeriupredoi merged commit f04aa17 into master Mar 19, 2021
@valeriupredoi valeriupredoi deleted the move_private_stock_cube_inside_cmorizer branch March 19, 2021 17:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants