-
Notifications
You must be signed in to change notification settings - Fork 40
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
Adds support for parametric vertical coordinate #528
Adds support for parametric vertical coordinate #528
Conversation
Thanks for taking this on! I've left a few high-level comments. Mostly I think it would be a lot clearer if we define a dataclass per transform that took care of the all the specialized logic: #528 (comment) |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #528 +/- ##
==========================================
+ Coverage 85.78% 93.35% +7.56%
==========================================
Files 13 13
Lines 2364 2197 -167
Branches 183 0 -183
==========================================
+ Hits 2028 2051 +23
+ Misses 303 146 -157
+ Partials 33 0 -33
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
ignore the 3.9 failures, I'll drop it soon. |
Thanks! This is getting close. Just some style nits. I haven't looked at the tests but I'm assuming you have it covered with some real life usage too? :) |
EDIT: I added a checklist to the top. |
Co-authored-by: Deepak Cherian <[email protected]>
for more information, see https://pre-commit.ci
Adds support for parametric vertical coordinates from CFConventions.
squeeze