Combine spatial data and xarray point obs

noaa-oar-arl / monet

The Model and ObservatioN Evaluation Toolkit (MONET)

https://monet-arl.readthedocs.io

Other

44 stars 20 forks source link

Combine spatial data and xarray point obs #97

Closed zmoon closed 1 year ago

zmoon commented 2 years ago

xref: https://github.com/NOAA-CSL/MELODIES-MONET/issues/115

new combine_da_to_da utility function
- currently used in MM aircraft branch here
~~.monet combine_da methods~~
- maybe later create a new combine method that could take xarray or df point data input
improve docstrings of some combine functions and .monet accessor methods

pep8speaks commented 2 years ago

Hello @zmoon! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

In the file monet/monet_accessor.py:

Line 1316:80: E501 line too long (102 > 79 characters) Line 1912:80: E501 line too long (102 > 79 characters)

In the file monet/util/combinetool.py:

Line 34:80: E501 line too long (99 > 79 characters) Line 130:80: E501 line too long (94 > 79 characters) Line 154:80: E501 line too long (109 > 79 characters) Line 190:80: E501 line too long (90 > 79 characters) Line 203:80: E501 line too long (96 > 79 characters) Line 204:80: E501 line too long (94 > 79 characters) Line 210:80: E501 line too long (94 > 79 characters) Line 216:80: E501 line too long (81 > 79 characters) Line 219:80: E501 line too long (120 > 79 characters) Line 244:80: E501 line too long (100 > 79 characters)

Comment last updated at 2022-01-20 00:05:40 UTC

zmoon commented 2 years ago

@bbakernoaa when you get a chance could you let me know what remains to be done here?

bbakernoaa commented 2 years ago

I don't think anything else needs to be done here. We have been using this in the MELODIES-MONET project so I think it should be ready to merge.

zmoon commented 2 years ago

@bbakernoaa in the new functions the docstring parameters listed don't match the signature in some cases, and for some I am not sure how to fix. Would you be able to have a quick go at that?

bbakernoaa commented 2 years ago

I'll try to take a look at it soon

zmoon commented 2 years ago

@bbakernoaa I made some edits. I am a little confused though: for combine_da it seems like the data argument should be a DataFrame, as it is being passed to combine_da_to_df in the df position, but the checks in combine_da make sure that it is an xarray Dataset. The combine_da signature seems to suggest the intention was for the new combine_da_to_da to be used instead?

bbakernoaa commented 2 years ago

@zmoon lets talk about this tomorrow as I'm a little confused

zmoon commented 2 years ago

@rschwant could you point to me how/where in MM you are using this?