DM-38358: V2: PlotPhotonTransferCurveTask can fail if the input dataset doesn't match expectations #176
Conversation
There was a problem hiding this comment.
Some docstring issues that should be resolved (not necessarily on this ticket).
If, as the JIRA ticket suggests, this should be run as a pipeline, it may be worth adding this to the PTC pipeline, so these plots are generated for every run (and are then available for use in notebooks). Neither of these issues feels like it needs to block.
| # var/mu vs mu corresponds to ptcPlot3 and the name | ||
| # matches when FITTYPE is FULLCOVARIANCE or | ||
| # [EXPAPPROXIMATION, POLYNOMIAL] | ||
| temp = figList1[2] |
There was a problem hiding this comment.
Can you explain this to me better? My interpretation is that instead of using the fResCov00 plot, this is now using the output of plotNormalizedCovariance, so this is swapping two plots of covariance. Is there a reason to that here instead of updating the plot functions to return what's needed?
I also notice that at least some plot functions do not have Returns entries in their docstrings, making it less clear what they're returning. Please add a ticket to fix that if you won't be doing so here.
There was a problem hiding this comment.
I made the change so that the third plot matches the name "normalizedVariance" in the two cases FULLCOVARIANCE vs [EXPAPPROXIMATION, POLYNOMIAL] . Both options use different functions. The latter is just one plotting function ("plot standard PTC") and returns only 3 plots. The former uses several functions, and the first function of these returns 5 plots, but its current third plot ("model residuals") did not match the 3rd plot of "plot standard PTC".
I'll change the docstrings, and add this task to the PTC pipeline, that's a good idea.
No description provided.