-
Notifications
You must be signed in to change notification settings - Fork 14
Transition pynio to cfgrib for wgrib2 engine. #263
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
base: main
Are you sure you want to change the base?
Conversation
Co-authored-by: Paul Madden <[email protected]>
maddenp-cu
left a comment
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.
Just a few more ideas, to take or leave.
I'll circle back tomorrow to your outstanding responses to my previous review.
maddenp-cu
left a comment
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.
Almost exclusively copy-editing suggestions, and only for text in diff regions or nearby -- I didn't go hunting in unchanged parts of files.
In addition to these, I'll note that there's a variety of keyword vs key word, and "Skew-T" is all over the place: "skewt", "SkewT", "Skew-T", "Skew T", etc. It could be worth (maybe someday?) normalizing these to make finding related code and comments easier. Many developers will think to do case-insensitive searching, but fewer will use regexes to make whitespace (much less hyphenation) optional. Just doing due diligence by pointing this out.
maddenp-cu
left a comment
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.
I think that's all I've got. I'm happy to respond to any @mentions and help with anything, and none of my comments should be considered blockers, just ideas/suggestions. Heroic effort here bringing this codebase up-to-date!
Replace mock tests with real tests.
Co-authored-by: Paul Madden <[email protected]>
Transition from the deprecated pynio to cfgrib for wgrib2 loading into an xarray object.
The need for this change is driven by a couple of immediate requirements:
As part of this extensive upgrade, I've also included additional tooling and unittests to help ensure quality. The current coverage is set to 75%. The intent is to follow up this PR with additional tests for 100% coverage.
New tools include:
All of these tools can be run with a new Makefile that includes helpful targets for a more automated experience with the test framework.