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-30301: Switch lsst.log usage to logging #552
Conversation
@@ -52,6 +52,8 @@ | |||
"SafeClipAssembleCoaddTask", "SafeClipAssembleCoaddConfig", | |||
"CompareWarpAssembleCoaddTask", "CompareWarpAssembleCoaddConfig"] | |||
|
|||
log = logging.getLogger(__name__.partition(".")[2]) |
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.
Slightly dangerous, as I think it fails catastrophically if the module name somehow doesn't contain .
.
Maybe safer to do __name__.replace("lsst.", "", 1)
?
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.
We do this all over the place in ctrl_mpexec (and elsewhere) -- we don't do it in daf_butler because in daf_butler the lsst.
is retained. I think we'd want to constrain the replacement to start of string as well.
A case could be made for a helper function to do this consistently in ctrl_mpexec and pipe_tasks but would that be pipe_base.task_logging? or lsst.daf.butler.core.logging that has it?
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.
I think by default I'm not going to change this. If ever this code moves out of lsst.
module then I'm sure more code fixes will be needed and it's going to break instantly. It isn't going to break though until the code moves.
@@ -39,6 +39,8 @@ | |||
|
|||
__all__ = ["MakeCoaddTempExpTask", "MakeWarpTask", "MakeWarpConfig"] | |||
|
|||
log = logging.getLogger(__name__.partition(".")[2]) |
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.
See above.
The package no longer directly uses the lsst.log package.
No description provided.