# DM-21221 broke cp_pipe due to lack of tests

XMLWordPrintable

## Details

• Type: Story
• Status: Done
• Resolution: Done
• Fix Version/s: None
• Component/s:
• Labels:
None
• Story Points:
2
• Team:
Data Release Production
• Urgent?:
No

## Description

The changing of ...Nl to ...nonLinearity is an API change which breaks things, including but not limited to makeBrighterFatterKernel due to a lack of tests (i.e. it's not really Andrés Alejandro Plazas Malagón's fault.

Also, the API change where that method doesn't return dataset and now just does return is breaking too (also due to lack of tests).

It is deliberately ambiguous whether this ticket is for fixing the breakages, or doing that and adding tests.

## Activity

Hide
John Swinbank added a comment -

I tried running the code, and agree that it runs.

However, I am confused about the ticket. It says:

the API change where that method doesn't return dataset and now just does return is breaking too

I don't understand why this would cause breakage. The only (non-test) place which calls fitPtcAndNonLinearity already has dataset in scope. What was the breakage here? Indeed, I made this change

 --- a/python/lsst/cp/pipe/ptc.py +++ b/python/lsst/cp/pipe/ptc.py @@ -354,7 +354,8 @@ class MeasurePhotonTransferCurveTask(pipeBase.CmdLineTask):  lookupTableArray = np.zeros((numberAmps, numberAduValues), dtype=np.float32)    # Fit PTC and (non)linearity of signal vs time curve, produce linearizer - dataset = self.fitPtcAndNonLinearity(dataset, lookupTableArray, ptcFitType=self.config.ptcFitType) + #dataset = + self.fitPtcAndNonLinearity(dataset, lookupTableArray, ptcFitType=self.config.ptcFitType)    if self.config.makePlots:  self.plot(dataRef, dataset, ptcFitType=self.config.ptcFitType) @@ -830,7 +831,7 @@ class MeasurePhotonTransferCurveTask(pipeBase.CmdLineTask):  dataset.nonLinearityResiduals[ampName] = linResidualNonLinearity  dataset.coefficientLinearizeSquared[ampName] = c0   - return dataset +# return dataset    def plot(self, dataRef, dataset, ptcFitType):  dirname = dataRef.getUri(datasetType='cpPipePlotRoot', write=True) 

and everything seems to work fine. Can somebody explain the issue?

Also also: we talked about how difficult this is to test. However, there are multiple calls to fitPtcAndNonLinearity in the tests directory. Any one (or all) of those could be checking the return value. Could we add that on this ticket?

Show
Hide
Merlin Fisher-Levine added a comment -

Given I couldn't run it at the time, those were guesses as to what I imagined might have been broken. If it was never broken, all the better. Sorry for not being explicit about what was assumed and what was tested.

Show
Merlin Fisher-Levine added a comment - Given I couldn't run it at the time, those were guesses as to what I imagined might have been broken. If it was never broken, all the better. Sorry for not being explicit about what was assumed and what was tested.
Hide
John Swinbank added a comment -

Looks good

Show
John Swinbank added a comment - Looks good
Hide
John Swinbank added a comment -

Oh, but since you haven't modified the tests as I suggested above, could you please file a new ticket to capture that before closing this one?

Show
John Swinbank added a comment - Oh, but since you haven't modified the tests as I suggested above, could you please file a new ticket to capture that before closing this one?
Hide
Andrés Alejandro Plazas Malagón added a comment -

OK!; DM-23681

Show
Andrés Alejandro Plazas Malagón added a comment - OK!; DM-23681

## People

• Assignee:
Andrés Alejandro Plazas Malagón
Reporter:
Merlin Fisher-Levine
Reviewers:
John Swinbank
Watchers:
Andrés Alejandro Plazas Malagón, Christopher Waters, John Swinbank, Merlin Fisher-Levine