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-33138: Add CTI correction to ip_isr #129

Merged
merged 12 commits into from Jul 26, 2022
Merged

DM-33138: Add CTI correction to ip_isr #129

merged 12 commits into from Jul 26, 2022

Conversation

czwa
Copy link
Contributor

@czwa czwa commented May 9, 2022

This change adds a task to construct a CTI dataset from the set of measurements created by ip_isr.

tests/test_deferredCharge.py Outdated Show resolved Hide resolved
pipelines/LsstCam/cpDeferredCharge.yaml Show resolved Hide resolved
python/lsst/cp/pipe/deferredCharge.py Show resolved Hide resolved
python/lsst/cp/pipe/deferredCharge.py Show resolved Hide resolved
python/lsst/cp/pipe/deferredCharge.py Outdated Show resolved Hide resolved
python/lsst/cp/pipe/deferredCharge.py Show resolved Hide resolved
python/lsst/cp/pipe/deferredCharge.py Outdated Show resolved Hide resolved
python/lsst/cp/pipe/deferredCharge.py Show resolved Hide resolved
python/lsst/cp/pipe/deferredCharge.py Outdated Show resolved Hide resolved
python/lsst/cp/pipe/deferredCharge.py Outdated Show resolved Hide resolved
overscan2 = data[:, 1]
test = (np.array(overscan1) + np.array(overscan2))/(nCols*np.array(signal))
testResult = np.median(test) > 5.E-6
self.log.info("Estimate of CTI test is %f for amp %s, %s.", np.median(test), ampName,
Copy link
Collaborator

Choose a reason for hiding this comment

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

I see three % on this line and only two variables after the string closes. This is why I hate this kind of formatting and think we've got it all wrong, as this sort of mistake can't happen with f-strings. Am I misreading this due to my inexperience with this type of formatting, or is that an error?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Oh god, it's the following string that's being added in with that last arg 😳

Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry for the noise, but this is why I hate this way of doing things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH, what I really wanted to write was testResult ? str1 : str2, but that's not valid python.

@czwa czwa merged commit 76d8c4c into main Jul 26, 2022
@czwa czwa deleted the tickets/DM-33138 branch July 26, 2022 21:52
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

2 participants