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

MNT: Change naming to MET as its plotting met data. #653

Merged
merged 4 commits into from
Apr 5, 2023

Conversation

zssherman
Copy link
Collaborator

@zssherman zssherman commented Apr 4, 2023

@zssherman zssherman requested a review from mgrover1 as a code owner April 4, 2023 16:11
@zssherman zssherman requested review from rcjackson and AdamTheisen and removed request for mgrover1 April 4, 2023 16:11
Copy link
Collaborator

@mgrover1 mgrover1 left a comment

Choose a reason for hiding this comment

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

One small suggestion

@zssherman
Copy link
Collaborator Author

Ooo yeah good point! Will fix that now

@zssherman
Copy link
Collaborator Author

@rcjackson Did pysp2 update anything that would change the waveform stats calculation?

@zssherman
Copy link
Collaborator Author

Seeing some unit changes etc recently to it.

@AdamTheisen
Copy link
Collaborator

@zssherman if we are changing this to just be a simple plot of the time series data, we can remove the LCL calculations from this example. I was somewhat thinking of this the other way around in that we should update the example to plot the SONDE time series with the LCL plotted in a plot of the altitude or something like that. I think that is what @rcjackson was maybe intending to do with this one. Thoughts?

@zssherman
Copy link
Collaborator Author

@AdamTheisen Ah makes sense! I can switch things around to be what was originally intended!

@zssherman
Copy link
Collaborator Author

@AdamTheisen Example has been updated for sonde data. Updating the unit tests now to match the pysp2 changes that are failing the CI.

@mgrover1
Copy link
Collaborator

mgrover1 commented Apr 4, 2023

@zssherman can you update those values in the test suite?

@zssherman
Copy link
Collaborator Author

@mgrover1 Done

@zssherman
Copy link
Collaborator Author

Confused on this class init failures for only python 3.8:
TypeError: init() got an unexpected keyword argument 'base'

@zssherman
Copy link
Collaborator Author

Ah so all tests failing for that base error are all older xarray version its looks like using base parameter in resample.

@zssherman
Copy link
Collaborator Author

zssherman commented Apr 4, 2023

@AdamTheisen @mgrover1 See other comments above. Do we drop 3.8 as we are three ahead with 3.9, 3.10 and 3.11 or do we find a work around for this base error in resampling.

@mgrover1
Copy link
Collaborator

mgrover1 commented Apr 4, 2023

Let's pin pandas<2.0... it is an issue on the xarray/pandas/flox side. We should consider pinning version in tests/such

@zssherman
Copy link
Collaborator Author

@mgrover1 Sounds like a plan!

@zssherman
Copy link
Collaborator Author

@mgrover1 I can't remember if that syntax is right for requirements.txt. I know it is for the .yml. Also two numpy installs removed one of them.

@zssherman
Copy link
Collaborator Author

And do you have the issue or where you found the info on the pandas/flox issue.

@mgrover1
Copy link
Collaborator

mgrover1 commented Apr 4, 2023

pydata/xarray#7716

@zssherman
Copy link
Collaborator Author

@mgrover1 Cool seems to be working! Just the usual stalled tests.

@zssherman
Copy link
Collaborator Author

zssherman commented Apr 4, 2023

@AdamTheisen @mgrover1 Once you both think this is good to merge, I can push it through

@zssherman zssherman merged commit ddbec03 into ARM-DOE:main Apr 5, 2023
@zssherman zssherman deleted the example_fix branch April 21, 2023 17:04
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.

Example Incorrect
3 participants