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
Review of DM-4373 (HSC backport: Add tract conveniences) #2
Conversation
@@ -72,3 +71,45 @@ class ExistingCoaddDataIdContainer(CoaddDataIdContainer): | |||
def makeDataRefList(self, namespace): | |||
super(ExistingCoaddDataIdContainer, self).makeDataRefList(namespace) | |||
self.refList = [ref for ref in self.refList if ref.datasetExists()] | |||
|
|||
class TractDataIdContainer(CoaddDataIdContainer): |
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.
Do we need this? It's not used anywhere in the stack and it comes with no tests, so it seems like we're just adding debt with no concrete gains.
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.
It is used several times in @PaulPrice's analysis script.
The use of deepCoadd (well, the "deep" part was configurable) was hard-coded, but we now write background-subtracted coadds as deepCoadd_calexp since HSC-1311. Changed CoaddDataIdContainer.makeDataRefList to respect the provided dataset type instead of always grabbing deepCoadd. However, the skymap doesn't have the same flexibility, and is always deepCoadd_skyMap, so simplified the parameter list. Conflicts: python/lsst/pipe/tasks/coaddBase.py This is essentially a cherry-pick of HSC-1366's: HyperSuprime-Cam/pipe_tasks@ea5e1ec Noting that the coaddDataIdContainer lives in coadd_utils in the LSST stack. Note also that HSC-1311 will be ported subsequently.
It is difficult to make a data reference that merely points to an entire tract: there is no data product solely at the tract level. This adds a TractDataIdContainer which generates a list of data references for patches within a given tract. Note that TractDataIdContainer was introduced in HSC's standalone commit: HyperSuprime-Cam/hscPipe@4a84654 It was added to HSC's python/hsc/pipe/tasks/stack.py task which does not exist in the LSST stack, so it was added to coadd_utils here in the file which also contains CoaddDataIdContainer.
0af7bbe
to
acbf38e
Compare
|
||
if "tract" in dataId: | ||
tractId = dataId["tract"] | ||
tractRefs = defaultdict(list) |
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.
You probably want to remove this line from the loop, and instead use it as a replacement for line 91.
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.
Nice catch! Fixed.
Make use of defaultdict and formatting updates.
acbf38e
to
5c0c45b
Compare
No description provided.