-
Notifications
You must be signed in to change notification settings - Fork 10
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
Package infrastructure update and other fixes #199
Conversation
I've fixed some header descriptions for |
@sbailey, pinging you for review. Note that there are some unresolved questions about new columns, see above. |
@sbailey: I think I found where So the definition would appear to be "If True, this exposure was used in the coadd of camera B,R,Z." When I was generating the zcatalog files for I was able to find references to Is it possibly a bug that |
Possibly more precise: "If True, this fiber in this exposure was used in the coadd of camera B,R,Z." |
Looks good for long form. For short form for FITS headers, maybe something like "fiber/exp used in coadd of camera B,R,Z" NUMEXP vs. COADD_NUMEXP and NUMTILE vs. COADD_NUMTILE: I only see the "COADD_*" versions of these in Kibo. If you ended up with all 4 columns when making daily zcatalogs, that's likely due to a leftover format when propagating upstream files before the FIBERMAP / EXP_FIBERMAP split. Please point me to an example output file that has all for columns and the command that generated it. I'm guessing that non-zero entries of NUMEXP and NUMTILE could be traced back to old data, and we'll need to add another special case to some reader to intercept and rename that. |
You should look in I hadn't done this previously, but there is some detective work to be done, because I'm seeing different survey/program having different columns. |
It appears that all flavors of ztile-*-dark-cumulative.fits have a NUMEXP column, but they also appear to be all zeros. I don't see from the code where this is appearing. Please trace upstream to the input files to see where this column first appears. Thanks. |
Sure, will do. |
I'm still working on tracing the origin of
It turns out this isn't actually true. I did a thorough analysis of all of the columns in the For the
Some of the targeting columns aren't present in all of the files, but I verified that they are missing where they are expected to be missing:
And there is a type mismatch for
For the
|
So far, I've only found |
It looks like zbest*.fits files have NUMEXP, and redrock*.fits files have COADD_NUMEXP. In that case I think the solution would be to intercept and rename NUMEXP as part of |
I will certainly do that, however, I am not convinced that |
At minimum I think it would be save to apply something like: if 'NUMEXP' in fibermap.colnames and 'COADD_NUMEXP' not in fibermap.colnames:
fibermap.rename_column('NUMEXP', 'COADD_NUMEXP')
if 'NUMTILE' in fibermap.colnames and 'COADD_NUMTILE' not in fibermap.colnames:
fibermap.rename_column('NUMTILE', 'COADD_NUMTILE') Then rerun desi_zcatalog and see if problems remain... |
Sure, I'll try that. |
@sbailey, assuming we completely get rid of |
This PR
setup.py
, etc.).pycodestyle
configuration.Remaining work:
NUMEXP
NUMTILE
IN_COADD_(B|R|Z)
.