-
Notifications
You must be signed in to change notification settings - Fork 29
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
New readers from MM satellite group #112
Conversation
* Update raqms.py tracking problems in colab * Update __init__.py * Update __init__.py * Update __init__.py * trying to make Kaggle import work * Update __init__.py * Update __init__.py * Update __init__.py * Update __init__.py * Update __init__.py * Update __init__.py * Update __init__.py * Update __init__.py * Update __init__.py * Update __init__.py * Update __init__.py * Update __init__.py * Update setup.cfg * Update setup.cfg * Update __init__.py * Update raqms.py adjusting timestamp. Needed for pairing. Unsure if this was a temporary fix sort of thing * Update raqms.py * Update raqms.py * Update raqms.py * Update omps_nadir.py * Update omps_nadir.py * data handling issue fix * Update omps_nadir.py * Update omps_nadir.py * add trimmed file reader for earthcube notebook * Update raqms.py * Update raqms.py * Update raqms.py * merge fixes * trying to fix for merge * Adding MOPITT read * tropomi no2 reader * rename * adding hdf close file * omps level 3 reader * Update raqms.py remove double of _fix_pres --------- Co-authored-by: mebruckner <48494069+mebruckner@users.noreply.github.com> Co-authored-by: Maggie Bruckner <mbruckner@raqms-ops.ssec.wisc.edu> Co-authored-by: Meng Li <mengli@MengM1.local>
@dwfncar @rrbuchholz do you have/want any tests for any of these? |
3.6 failing due to #114 , but there are also some comments from flake8 to address, and some formatting stuff. |
I have an example Jupyter notebook for MOPITT in the MELODIES MONET develop_satellite branch that needs to be pulled. |
@rrbuchholz that'll be great to have those MM examples, but I was referring here to smaller test cases for the MONETIO readers to add to the MONETIO test suite. It's ok if we don't have, just thought I would ask. |
Data file @rrbuchholz shared now available at https://csl.noaa.gov/groups/csl4/modeldata/melodies-monet/data/example_observation_data/satellite/MOP03JM-201701-L3V95.9.3.he5 |
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.
Some initial comments.
text formatting differences Co-authored-by: Zachary Moon <zmoon92@gmail.com>
text formatting using consistent language Co-authored-by: Zachary Moon <zmoon92@gmail.com>
using consistent language in text descriptions Co-authored-by: Zachary Moon <zmoon92@gmail.com>
Code semantics Co-authored-by: Zachary Moon <zmoon92@gmail.com>
Remove development debugging Co-authored-by: Zachary Moon <zmoon92@gmail.com>
Remove development debugging Co-authored-by: Zachary Moon <zmoon92@gmail.com>
with (somewhat) informative message containing the options
ds.attrs["quality_flag"] = varname | ||
ds.attrs["quality_thresh_min"] = variable_dict[varname]["quality_flag_min"] |
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.
These are dataset attrs so they will be overwritten if there is more than one variable in variable_dict
.
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.
True. This is an old version of the TROPOMI reader. I add a new dataset attribute 'var_applied' to define the variables list for each quality flag in the new reader
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.
@mlirenzhenmayi I don't see this in the current version of the code.
values[quality_flag <= quality_thresh_min] = np.nan | ||
|
||
|
||
def open_dataset(fnames, variable_dict, debug=False): |
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.
The variable_dict
is necessary for the fn to be useful. Is this something that will always be the same? If so, could include in the code sort of like in the MOPITT one.
and/or document in the docstring the format
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.
fn always change with the date and vary one by one. It seems that there's no string always the same.
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.
By the way, by "fn" I meant function, not filename. I was wondering if we could set it up so you could pass varnames
like the MOPITT one, instead of the variable dict. The variable dict info could be embedded in the module, if it is fixed for a certain variable.
Hi @zmoon! Just wondering what you need from us in the satellite team to get this pull request merged? Would be great to try and have it done before the summer release of MELODIES MONET. (It's been a year since we opened this!) |
Hi @rrbuchholz , very sorry, it's been on my list to get back to @mlirenzhenmayi |
this way the current test still works
seems to be more likely what we want though I'm not sure why setting 'quality_flag' to the var name is helpful
@rrbuchholz and @mlirenzhenmayi I added some basic tests for the other two. I also made a few changes here and there (e.g. adding attributes to variables), maybe you can check that your workflows still work and/or review. |
It all looks good to merge. |
@rrbuchholz thanks for checking it out. I haven't added @mlirenzhenmayi 's updates to tropomi but maybe it's simpler to merge this now and add that after. |
@zmoon Yeah, I think that sounds like a good idea. |
Adding in:
Tropomi L2 reader
OMPS L3 reader
MOPITT L3 reader
and adding to
__init__.py
file