-
Notifications
You must be signed in to change notification settings - Fork 14
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
Upgrade Earlinet reader and implement vertical profiles #855
Conversation
Some stations have netcdf with multiple observations (timesteps). We currently kick them out. Could think of a more elegant solution, but that would need some input from others |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please address the small change I commented on and merge then. No need for a new review.
pyaerocom/colocation_3d.py
Outdated
if regrid_res_deg is not None: | ||
data = _regrid_gridded(data, regrid_scheme, regrid_res_deg) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if these lines are nor covered by test, maybe the if
clause should be marked with # pragma: no cover
pyaerocom/colocation_3d.py
Outdated
if use_climatology_ref: | ||
col_freq = "monthly" | ||
obs_start = const.CLIM_START | ||
obs_stop = const.CLIM_STOP |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seem my prebious comment about not covered lines
TEST_FILE = [ | ||
ROOT / "earlinet_example_for_ci.pkl", | ||
] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not convinced that pickeø files will work across different versions of Python and numpy/netCDF4
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this is a concern. Either we find a more robust way of storing the UngriddedData
object (not obvious to me now, would require some research), or I write a fixture to create this UngriddedData
object on the fly. I'm happy to open a PR to patch this after the fact if you are okay merging for now
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My main concern is the use of pickle files on testing.
This works now, but there is not guaranty that it will continue to work with new version of Pyhon/numpy/NetcDF4/....
Need to upgrade the EARLINET reader to the new format, and move VerticalProfiles through the rest of the pyaerocom pipeline to Aeroval.