Skip to content
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-18051: Get defect machinery working for the AuxTel #91

Merged
merged 12 commits into from Apr 3, 2019

Conversation

mfisherlevine
Copy link
Contributor

No description provided.

@timj timj changed the title tickets/DM-18051 DM-18051: Create defects file for AuxTel Mar 28, 2019
Copy link
Contributor

@SimonKrughoff SimonKrughoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Except for what looks like syntax error in the defect lookup function, I'm good with this.

bin.src/genDefectFits.py Outdated Show resolved Hide resolved
bin.src/genDefectRegistry.py Show resolved Hide resolved
config/auxTel/flat.py Show resolved Hide resolved
python/lsst/obs/lsst/lsstCamMapper.py Show resolved Hide resolved
@timj
Copy link
Member

timj commented Mar 29, 2019

Am I right in thinking that a bunch of this code is copied directly from obs_subaru? At first glance that doesn't seem like a sustainable approach to development so why are we copying code around rather than putting it in obs_base or somewhere?

config/auxTel/dark.py Outdated Show resolved Hide resolved
@timj
Copy link
Member

timj commented Mar 29, 2019

Please remove the bad merge commit on this branch.

@mfisherlevine
Copy link
Contributor Author

Am I right in thinking that a bunch of this code is copied directly from obs_subaru? At first glance that doesn't seem like a sustainable approach to development so why are we copying code around rather than putting it in obs_base or somewhere?

Yes, that's correct, both that it was copied, and that it's not sustainable. However, it's a pattern that shouldn't be followed in general, so was decided that it wouldn't be moved to obs_base precisely for that reason, i.e. to discourage others from doing this. Once we live in the glorious world of Gen3 and post-calibration-product-production-overhaul things like this will be cleaned up/stop happening.

@mfisherlevine mfisherlevine force-pushed the tickets/DM-18051 branch 3 times, most recently from f3d1895 to 7dfeba3 Compare April 3, 2019 14:36
@mfisherlevine mfisherlevine changed the title DM-18051: Create defects file for AuxTel tickets/DM-18051: Get defect machinery working for the AuxTel Apr 3, 2019
@timj timj changed the title tickets/DM-18051: Get defect machinery working for the AuxTel DM-18051: Get defect machinery working for the AuxTel Apr 3, 2019
Ignore the output .fits defect files
Ignore the defectRegistry
Add a SConscript file which calls genDefectFits.py
and genDefectRegistry.py to build fits files from the
defect.dat files, and create a defect registry at
scons-time.

This is following the pattern (and copying the code)
in obs_subaru.

It is deliberately not moving this code into obs_base
nor implementing this for other parts of obs_lsst
as this is a pattern we want to change, but there is no
point in doing this before Gen3 or fixing up calibration
stuff in general.
Change the key used
Change the keys stored in the registry to be careful
Store both the name and number of the detector in the
defect fits file
Do this in auxTel.py so that it applies everywhere
Also, fix a PEP8 violation in the spacing while there
@mfisherlevine mfisherlevine merged commit 46b2492 into master Apr 3, 2019
@timj timj deleted the tickets/DM-18051 branch May 12, 2022 16:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants