DM-39996: Code modernization and ruff configuration #462
Conversation
| "Call Registry.expandDataId before Butler.get to avoid this.", | ||
| # It is not entirely obvious what the correct stacklevel | ||
| # should be for a formatter. Outside of obs_base will be | ||
| # the butler datastore. Outside of daf_butler might be | ||
| # the right answer and could be the caller using butler.get | ||
| # or could be a runQuantum method. |
There was a problem hiding this comment.
If we cannot be sure where the actual cause is located, would it be better to print a full traceback?
There was a problem hiding this comment.
Maybe I leave it warning from the formatter. @TallJimbo do you have an opinion on where a warning from a formatter should appear to be coming from?
There was a problem hiding this comment.
We definitely don't want to fully lose any of the traceback, but if we have any control over what users see most prominently, it's probably whatever low-level I/O code the formatter is calling (e.g. if it's coming from cfitsio, I'd want to see cfitsio's error message).
There was a problem hiding this comment.
This question is specifically about what line and file this warning is reported as coming from. For a formatter that was not directly called by the user maybe the formatter is the right source of the warning message. The only other alternative would be the place that called butler.get.
There was a problem hiding this comment.
Ah, sorry, didn't look closely enough. For this particular warning, whatever called QuantumContext.get would be ideal.
There was a problem hiding this comment.
I could extend find_outside_stacklevel a bit. My initial thought was to takes *args and so use it here with:
stacklevel=find_outside_stacklevel("lsst.obs.base", "lsst.daf.butler")and then it would be reported as coming from QuantumContext.get in the pipeline execution environment. At least it would come from the user code if they called butler.get(). Making it come from the caller of QuantumContext.get seems a bit trickier. One option is to add lsst.pipe.base.QuantumContext to the list as well and so be able to filter out classes. I can give that a try but it would have to be on a new ticket. For now then I'll merge this and then come back later to fix it up (I'll add a TODO ticket).
There was a problem hiding this comment.
Deferring is completely fine. This particular warning should be sufficiently rare that if somebody asks on Slack every time they see it and I have to answer them it still won't be much of a burden, so on its own I don't think this is sufficient motivation for big enhancements to find_outside_stacklevel (though I agree that might provide useful elsewhere).
What should the stacklevel really be though? It should probably be outside of obs_base and daf_butler and this will require a slight modification to find_outside_stacklevel to take *args.
This appeases bugbear linting by showing that you know that the property access will be doing something.
If a class has no abstract methods making it abstract with "abc" is pointless.
This will let daf_butler and pipe_base references resolve.
Co-authored-by: Andy Salnikov <[email protected]>
Checklist
doc/changes