-
Notifications
You must be signed in to change notification settings - Fork 12
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
Fix Level issue #195
Fix Level issue #195
Conversation
@EdwardSafford-NOAA any chance you can fix the unrelated issue that is causing the CI to fail |
Having a look now. |
…into bug/level-194
@CoryMartin-NOAA @EdwardSafford-NOAA This is the exact same issue I just debugged this morning. It is due to the fact that EMCPy's required matplotlib build is outdated. I updated in my PR to the newest version and it solved the CI issues. This may be related to what is happening on WCOSS2 as well and mentioned it to Cory this morning. |
Yes I spotted that too. I updated matplotlib to version 3.9.0 (latest afaik) in requirements.txt. That does not seem to have fixed the CI test though, so maybe there's more I need to do? |
Oh, I see additional places. Lemme fix that and retry. |
Not sure why I'm getting these, but a bunch of emails have come to me with the following message:
Apparently, we've got something watching the repos and trying to enforce git flow naming conventions. I think the watchdog wants the branch to be named Thanks! |
Thanks Steve, yeah I'm getting the same automated emails. @EdwardSafford-NOAA just an FYI for future reference |
@CoryMartin-NOAA I think I messed up something in my attempts to fix the CI issue. It looks like my change to |
@EdwardSafford-NOAA I think I may have already merged that into develop, perhaps this branch just needs made up to date with develop? |
@CoryMartin-NOAA Ok, I think I get it. You did merge my 1 char change and that is in develop now. My develop is current, and my bug[fix]/level-192 has develop merged into it. So everything is copacetic except for the ongoing CI issue. |
With the level fix now in develop I'm going to close this PR. The CI error has been resolved by updating the emcpy hash and the matlabplot version. |
Stupidly simple fix, but took forever to find owing to a seriously unhelpful error message.
This fix enables conventional
ps
type plots to work correctly. Unlike other conventional time-series data,ps
has only one level (surface, all others have 13). With the check on levels > 1 resulted in trying to assign coordinates of None for the dataset's Level dimension. Adding to the confusion the same error resulted from two different operations (assign_coordinates()
and then amerge()
of the date/time values). The actual error message wasValueError: dimension 'Level' already exists as a scalar variable
, which really didn't describe the situation at all.Closes #194 .