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

Combine spatial data and xarray point obs #97

Merged
merged 12 commits into from
Dec 20, 2022
Merged

Combine spatial data and xarray point obs #97

merged 12 commits into from
Dec 20, 2022

Conversation

zmoon
Copy link
Member

@zmoon zmoon commented Jan 19, 2022

xref: NOAA-CSL/MELODIES-MONET#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

This comment has been minimized.

@zmoon
Copy link
Member Author

zmoon commented Feb 1, 2022

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

@bbakernoaa
Copy link
Member

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
Copy link
Member Author

zmoon commented Feb 1, 2022

@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
Copy link
Member

I'll try to take a look at it soon

@zmoon
Copy link
Member Author

zmoon commented Feb 7, 2022

@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
Copy link
Member

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

@zmoon
Copy link
Member Author

zmoon commented Apr 1, 2022

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

@zmoon zmoon changed the title Combine spatial data and point obs Combine spatial data and xarray point obs Dec 20, 2022
@zmoon zmoon marked this pull request as ready for review December 20, 2022 04:06
@zmoon zmoon merged commit a759ee4 into develop Dec 20, 2022
@zmoon zmoon deleted the dev-combine_to_da branch December 20, 2022 04:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants