-
Notifications
You must be signed in to change notification settings - Fork 9
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
Remove support for pfr<=0.9.x #170
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #170 +/- ##
==========================================
- Coverage 96.46% 95.93% -0.53%
==========================================
Files 15 14 -1
Lines 509 492 -17
==========================================
- Hits 491 472 -19
- Misses 18 20 +2 ☔ View full report in Codecov by Sentry. |
57204b6
to
5cf7b3a
Compare
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.
@cisaacstern It occurs to me you might have an opinion here. I'm for pulling the trigger and moving things forward as i think there are further issues with the ways recipes are handled that will require potential further deprecation or at least heavy guidance on the semantics of the two approaches - PTransform construction vs lazily building our full pipeline inside a function that will serve as the recipe (this is enabled by the PR about lazy pipeline construction #168) |
Thanks for the ping I will take a look this afternoon. |
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.
Overall LGTM, @moradology, thanks so much for doing this overdue maintenance! One other thing we should remove as part of this PR is:
pangeo-forge-runner/pangeo_forge_runner/storage.py
Lines 110 to 115 in 0eda0ca
class MetadataCacheStorage(StorageTargetConfig): | |
""" | |
Storage configuration for caching metadata during recipe baking | |
""" | |
pangeo_forge_target_class = "MetadataTarget" |
Since that is only relevant in pre-0.10.0 releases.
Apart from that, I haven't taken a close look yet as to why codecov is upset, but if possible would be great to get that passing.
for more information, see https://pre-commit.ci
well, codecov was complaining b/c the number of removals seemed to reduce the overall ratio of coverage, not b/c important things changed. I added some gonna merge this sucker |
Awesome! Thanks all! |
This draft aims to move us towards cutting support for the older
to_beam
recipe version. This ought to simplify developer work and cut down on CI complexity. #144