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
DM-31924: deprecate aspects of the data ID packing system #820
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #820 +/- ##
==========================================
- Coverage 87.77% 87.68% -0.10%
==========================================
Files 268 268
Lines 35032 35066 +34
Branches 7375 7383 +8
==========================================
- Hits 30748 30746 -2
- Misses 3125 3162 +37
+ Partials 1159 1158 -1
☔ View full report in Codecov by Sentry. |
ae6d33a
to
5fb0611
Compare
Codecov is sad only because it can't tell that the code that lost coverage is now deprecated (and I don't think it's worth keeping the tests alive until the removal happens). |
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.
Is the "remove" ticket already filed? I've found it very handy to mark the methods that need to be removed post-deprecation with # DM-XXXXX: remove
, especially if their deprecation is implicit, like some of these appear to be.
It is (DM-38687), but it hadn't been when I made most of these changes. I've gone back and added references to it. |
31bc19e
to
bb53012
Compare
dimensions=subconfig["dimensions"], | ||
) | ||
with warnings.catch_warnings(): | ||
# Don't warn when deprecated code calls other deprecated code. |
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.
Does this method (_extractPackerVisitors
) need a # TODO: Remove this class on DM-38687.
comment?
This also moves test coverage for what will remain (here) of the dimension packer system to test_dimensions.py, relying on a new simpler concrete test packer. All of the production concrete packers will be defined in downstream packages going forward, and the machinery for constructing them from a DimensionUniverse is being deprecated, so all that remains to be tested is the DimensionPacker ABC itself.
Most other interfaces allow pure kwargs as data IDs, and we can also supply the "fixed" dimension values as a default.
2e0fd7a
to
d843b29
Compare
Checklist
doc/changes