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-42927: Update cp_verify connections/classes/outputs for analysis_tools #237

Merged
merged 10 commits into from
May 1, 2024

Conversation

czwa
Copy link
Contributor

@czwa czwa commented Apr 12, 2024

This updates the handling of calibration products. I've renamed some classes and files to be more informative. This also updates the GridPlot code to fix the column-vs-column plotting.

@czwa czwa requested a review from sr525 April 12, 2024 22:37
@czwa czwa force-pushed the tickets/DM-42927 branch 2 times, most recently from 569a7d9 to 47f83fa Compare April 18, 2024 21:03
# atools.defectsBadColumns.produce.plot.zAxisLabel: "Number of Hot Pixels"
# atools.defectsBadColumns.quantityKey: DEFECTS_DEFECTS_DIST_N_BAD_COLUMNS
# atools.defectsBadColumns.unit: "---"
python: |
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should all these commented out bits be here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've left commented blocks that have associated JIRA tickets. I think in most of these cases, the commented code will work once those tickets merge (and hopefully un-comment these blocks).

ylabel = Field[str](
doc="String argument passed into fig.supylabel() defining the figure y label.",
optional=True,
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we make these xAxisLabel and yAxisLabel to bring them inline with other plots?

if xList is not None:
namedKey = self.panels[index].plotElement.xKey
newData[namedKey] = data[xList[i]]
if key in xList:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know that you didn't write most of this section but some comments in here would be really useful.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Documented as best I could since I'll likely need to work on this again. There are multiple levels of indirection that make this tool difficult to use.

2: "PTC_COV_01",
3: "PTC_COV_02",
4: "PTC_COV_11",
5: None,
Copy link
Collaborator

Choose a reason for hiding this comment

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

What does the 5: None do?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Grid plot needs to be able to generate an N*M grid of panels, but I only need 5, and 5 is unfortunately prime. By setting this to None, no columns will match, and the sixth panel will be empty.

Copy link
Contributor Author

@czwa czwa left a comment

Choose a reason for hiding this comment

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

I hit the wrong button, so this is just me adding my additional comments.

if xList is not None:
namedKey = self.panels[index].plotElement.xKey
newData[namedKey] = data[xList[i]]
if key in xList:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Documented as best I could since I'll likely need to work on this again. There are multiple levels of indirection that make this tool difficult to use.

2: "PTC_COV_01",
3: "PTC_COV_02",
4: "PTC_COV_11",
5: None,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Grid plot needs to be able to generate an N*M grid of panels, but I only need 5, and 5 is unfortunately prime. By setting this to None, no columns will match, and the sixth panel will be empty.

@czwa czwa force-pushed the tickets/DM-42927 branch 4 times, most recently from 1f88af9 to d3f9077 Compare April 30, 2024 22:29
@czwa czwa merged commit 6a935c4 into main May 1, 2024
8 checks passed
@czwa czwa deleted the tickets/DM-42927 branch May 1, 2024 23: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