GCR reader for DC2 static coadd catalogs #157
Conversation
|
The LSST Project refers to its 'y' filter as lower-case, and I presume this will be the case for distributed data products as well. |
|
HSC also calls it y. Y is usually reserved for the HgCdTe c. 1 micron band. |
|
Expand patches back out as strings. In the LSST current skymap scheme patches are strings. E.g., '0,4', 1,2' or '2,3'. It is often convenient to represent them as '04', '12', or '23', but those shouldn't be translated back into ints of 4, 12, 23. Such ints are of course unique, but it's less obvious how to get all of the patches in a given row or column and makes the neighbor relationship less clear. |
|
thanks for the feedback! |
| self.base_dir = base_dir or '/global/projecta/projectdirs/lsst/global/in2p3/Run1.1-test2/summary/' | ||
| assert os.path.isdir(self.base_dir), '{} is not a valid directory'.format(self.base_dir) | ||
| self._datasets, self._columns = self._generate_native_datasets_and_columns(self.base_dir) | ||
| assert self._datasets, 'No catalogs were found in {}'.format(self.base_dir) |
There was a problem hiding this comment.
I'd prefer a try/except to these two asserts. An assert should generally be reserved for testing. Not finding a catalog because you mistyped a directory path shouldn't trigger an assertion. Or put another way, if you remove all of the asserts from code it should run the same way.
| 'dec': 'coord_dec', | ||
| } | ||
|
|
||
| for band in 'ugrizY': |
There was a problem hiding this comment.
Lower-case y
for band in 'ugrizy':
Capitalization of y-band filters has certainly drifted around, but at least for the past few years the LSST Project has been pretty consistent internally and externally about listing 'y'.
| @@ -10,36 +10,45 @@ | |||
|
|
|||
| class DC2StaticCoaddCatalog(BaseGenericCatalog): | |||
|
|
|||
| _native_filter_quantities = {'tract', 'patch'} | |||
| _native_filter_quantities = {'tract', 'patch_ra', 'patch_dec'} | |||
There was a problem hiding this comment.
I don't like patch_ra and patch_dec, although I'm fine with dividing them up. There's nothing that says that patches have to be oriented along RA or Dec.
There was a problem hiding this comment.
Do you need an int lookup for something? Is there a reason you can't just have
_native_filter_quantities = {'tract', 'patch'}
but have patch be a str?
There was a problem hiding this comment.
I don't think it's programatically important to be able to iterate over patches in row, column. I just want the patch value to match the name the user will see in loading an image or otherwise looking up data by tract, patch.
There was a problem hiding this comment.
@wmwv it surely can. I've updated per your suggestions. The only downside is that people cannot do (lambda p: p < 2, 'patch_ra') as a native filter, but I agree with you that this is probably not a common use case.
|
@yymao Could you merge this in in advance of the DESC Seminar tomorrow (May 30)? |
|
Sure. If there's no objection I'll merge this tonight. |
This PR adds a GCR reader for the DC2 static coadd catalogs generated by @wmwv in #153 and also an example notebook of how to use it.