DM-40367: Add connection name to deprecation warning #367
Conversation
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #367 +/- ##
==========================================
- Coverage 84.07% 84.06% -0.01%
==========================================
Files 77 77
Lines 9075 9082 +7
Branches 1739 1740 +1
==========================================
+ Hits 7630 7635 +5
- Misses 1159 1160 +1
- Partials 286 287 +1
☔ View full report in Codecov by Sentry. |
python/lsst/pipe/base/connections.py
Outdated
| @@ -372,7 +372,9 @@ def __call__(cls, *, config: PipelineTaskConfig | None = None) -> PipelineTaskCo | |||
| for obj in instance._allConnections.values(): | |||
| if obj.deprecated is not None: | |||
| warnings.warn( | |||
| obj.deprecated, FutureWarning, stacklevel=find_outside_stacklevel("lsst.pipe.base") | |||
| f"{obj.name} (from {obj._deprecation_context}): {obj.deprecated}", | |||
There was a problem hiding this comment.
Instead of obj.name here, I think we want to iterate over _allConnections.items() and use the key from that mapping. Currently this is the dataset type name, which is not unique, and the mapping key is the task-internal connection name, which is.
There was a problem hiding this comment.
Maybe if we had that info I wouldn't need to find the filename and line number where it was originally defined because the content would be unique enough that it could be found from searching the source.
There was a problem hiding this comment.
I think the combination of the two would be good in addition to the work you did here for maximum info. something like "connection {dict key} with dataset type {obj.name} is deprecated....."
There was a problem hiding this comment.
That was easy. Now it looks like:
FutureWarning: Connection finalizedPsfApCorrCatalog with datasetType finalVisitSummary (from /Users/timj/work/lsstsw/stack/lsst-scipipe-7.0.0/Darwin/pipe_tasks/g85f8d04a89+f020c27613/python/lsst/pipe/tasks/makeWarp.py:121): Deprecated in favor of 'visitSummary'. Will be removed after v26.
python/lsst/pipe/base/connections.py
Outdated
| @@ -372,7 +372,9 @@ def __call__(cls, *, config: PipelineTaskConfig | None = None) -> PipelineTaskCo | |||
| for obj in instance._allConnections.values(): | |||
| if obj.deprecated is not None: | |||
| warnings.warn( | |||
| obj.deprecated, FutureWarning, stacklevel=find_outside_stacklevel("lsst.pipe.base") | |||
| f"{obj.name} (from {obj._deprecation_context}): {obj.deprecated}", | |||
There was a problem hiding this comment.
I think the combination of the two would be good in addition to the work you did here for maximum info. something like "connection {dict key} with dataset type {obj.name} is deprecated....."
This provides some context. Also remove the find_outside_stacklevel because there is nothing in the stack trace that is relevant to the deprecation warning.
This is then used in the warning message emitted later.
This provides some context. Also remove the find_outside_stacklevel because there is nothing in the stack trace that is relevant to the deprecation warning.
Depends on lsst/utils#168
Checklist
doc/changes