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-32817: Switch from ButlerURI to ResourcePath #623
Conversation
Codecov Report
@@ Coverage Diff @@
## main #623 +/- ##
==========================================
- Coverage 84.18% 84.08% -0.11%
==========================================
Files 237 237
Lines 30327 30014 -313
Branches 5015 4990 -25
==========================================
- Hits 25532 25238 -294
+ Misses 3652 3638 -14
+ Partials 1143 1138 -5
Continue to review full report at Codecov.
|
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.
Every single one of my comments is just a point where we should be using the new ResourcePathExpression
instead of something like Union[str, ResourcePath]
(to pick up pathlib.Path
now, and automatically pick up new types we could coerce in the future).
Most of those are patch commits, but note that I haven't tried to add the ResourcePathExpression
imports those will need, and I also haven't tried to fix the many docstrings - the latter just because I don't have solid of a plan for what to change them to.
This code is tested in ResourcePath unit tests. The remaining tests demonstrate that the ButlerURI compatibility code is working.
ButlerURI is still available for now.
numpydoc has no guidance for how to handle a long parameter type line. Rearrange things slightly to move the type information to the parameter documentation instead. See numpy/numpydoc#87
Co-authored-by: Jim Bosch <jbosch@astro.princeton.edu>
ResourcePathExpression docstring is not overly helpful.
I can't work out how to attach a deprecation message to ButlerURI. The problem is that ResourcePath only works for ResourcePath or specific subclasses (trying to inject a ButlerURI class into the class hierarchy doesn't work because it triggers the subclass code in ResourcePath). Aliasing ResourcePath to ButlerURI works fine but adding a deprecation message ends up either adding a deprecation message to ResourcePath itself (because they are aliases) or adding a deprecation message to the ButlerURI constructor (so class methods no longer work). |
I can't think of a way to add a deprecation message, either. |
Straightforward replacement with a couple of extra clean up commits.
I will see if I can get ButlerURI to give some kind of deprecation warning.
Checklist
doc/changes