DM-26393: Add Object.yaml specification #247
Conversation
| import os | ||
| from lsst.utils import getPackageDir | ||
|
|
||
| config.filterMap = {band: band for band in 'ugrizy'} |
There was a problem hiding this comment.
Does this need to know about the new filter names for imsim?
There was a problem hiding this comment.
Don't think so. As of w_2020_32, we were still calling them ugrizy:
/datasets/DC2/repoRun2.2i/rerun/w_2020_32/DM-26287/coadd/deepCoadd
[yusra@lsst-devl01 deepCoadd]$ ls
g i r u y z```
There was a problem hiding this comment.
Ah sorry. I saw "filterMap" and thought this was talking about filters. I missed the band on the right. Is this really bandMap ?
Note that the physical filter for imsim did change recently to include the sims throughputs version number.
There was a problem hiding this comment.
Looks good. Only two comments:
- Please "ra", "dec" instead of "Ra", "Decl".
- Why specify
priorityListagain? It is whatever it was from previous steps. But I also don't understand why it's even ever used inwriteObjectTable.py.
I believe I made the same comments on the obs_subaru Object.yaml and lost and expect to lose again. The priorityList is just an internal issue, I don't deeply care.
- But the "ra", "dec" naming issue means that DESC will literally rewrite the files again or run a branch with different choices to standardize the columns names for DESC DC2. I really encourage DRP+Science Platform to revisit this issue and choose this standardization.
[I can't officially set a status here because I created the PR in order to comment on it.]
| from lsst.utils import getPackageDir | ||
|
|
||
| config.filterMap = {band: band for band in 'ugrizy'} | ||
| config.functorFile = os.path.join(getPackageDir("obs_lsst"), 'policy', 'imsim', 'Object.yaml') |
There was a problem hiding this comment.
'obs_lsst' instead of "obs_lsst" to be quote-character consistent
config/imsim/writeObjectTable.py
Outdated
| @@ -0,0 +1 @@ | |||
| config.priorityList = ['i', 'r', 'z', 'y', 'g', 'u'] | |||
There was a problem hiding this comment.
Where is priorityList used in the WriteObjectTable class?
Isn't the priorityList already determined by mergeCoaddDetections/Measurements.py?
If this priorityList and mergeCoaddDetections/Measurements.py disagree are things still consistent? If it doesn't matter, then what does priorityList do for WriteObjectTable.py
I understand that obs_subaru has the same config.priorityList setting for writeObjectTable.py.
There was a problem hiding this comment.
Vestigial config. Good catch. To be removed on this ticket on its own commit.
| # coord_dec because "dec" is reserved in most SQL DBs | ||
| functor: DecColumn | ||
| dataset: ref | ||
| Ra: |
There was a problem hiding this comment.
"RA" or "ra". My preference would be "ra".
While one sees "Dec", "dec", and sometimes "DEC"; I've never seen "Ra" -- that just makes me think of the Sun god.
There was a problem hiding this comment.
See convo on Jira. Any column with dataset !=ref gets expanded by filter: uRa, gRa, iRa...
| functor: CoordColumn | ||
| args: coord_ra | ||
| dataset: meas | ||
| Decl: |
There was a problem hiding this comment.
That's annoying. Can we make this "dec" instead of "decl"?
I understand some ancient databases get confused with "dec"imal type, but Parquet certainly doesn't care and Postgres doesn't get confused. Is there something we're using where this is a problem?
There was a problem hiding this comment.
Where "annoying" specifically means I will literally have to rewrite out the DC2 Object files again with a column named "dec" before providing to DESC members.
There was a problem hiding this comment.
Is there something we're using where this is a problem?
qserv/mysql.
I don't know what to say. I put this ticket on the shelf hoping an idea would appear for giving you an ra/dec here, but I think the best I can offer is a ra/decl with the same values coord_ra/coord_dec.
yalsayyad
force-pushed
the
tickets/DM-26393
branch
from
f14b325
to
9da1475
Compare
config/imsim/writeObjectTable.py
Outdated
| @@ -0,0 +1 @@ | |||
| config.priorityList = ['i', 'r', 'z', 'y', 'g', 'u'] | |||
There was a problem hiding this comment.
Vestigial config. Good catch. To be removed on this ticket on its own commit.
| import os | ||
| from lsst.utils import getPackageDir | ||
|
|
||
| config.filterMap = {band: band for band in 'ugrizy'} |
There was a problem hiding this comment.
Don't think so. As of w_2020_32, we were still calling them ugrizy:
/datasets/DC2/repoRun2.2i/rerun/w_2020_32/DM-26287/coadd/deepCoadd
[yusra@lsst-devl01 deepCoadd]$ ls
g i r u y z```
| # coord_dec because "dec" is reserved in most SQL DBs | ||
| functor: DecColumn | ||
| dataset: ref | ||
| Ra: |
There was a problem hiding this comment.
See convo on Jira. Any column with dataset !=ref gets expanded by filter: uRa, gRa, iRa...
| functor: CoordColumn | ||
| args: coord_ra | ||
| dataset: meas | ||
| Decl: |
There was a problem hiding this comment.
Is there something we're using where this is a problem?
qserv/mysql.
I don't know what to say. I put this ticket on the shelf hoping an idea would appear for giving you an ra/dec here, but I think the best I can offer is a ra/decl with the same values coord_ra/coord_dec.
yalsayyad
force-pushed
the
tickets/DM-26393
branch
2 times, most recently
from
c630855
to
d789d4d
Compare
yalsayyad
force-pushed
the
tickets/DM-26393
branch
from
d789d4d
to
cb21250
Compare
yalsayyad
force-pushed
the
tickets/DM-26393
branch
2 times, most recently
from
e6ad764
to
4de0473
Compare
to transform multiband outputs to the parquet Object Table
yalsayyad
force-pushed
the
tickets/DM-26393
branch
from
4de0473
to
de48ce8
Compare
to transform multiband outputs to the parquet Object Table