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.

Sign up for GitHub

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

Jump to bottom

DM-44085-hotfix: Make skymap optional in makePlot for ScatterPlotWithTwoHists #245

Merged
merged 1 commit into from
Apr 30, 2024
Merged

DM-44085-hotfix: Make skymap optional in makePlot for ScatterPlotWithTwoHists #245

merged 1 commit into from
Apr 30, 2024

Conversation

arunkannawadi
Copy link
Member

No description provided.

@arunkannawadi
Make skymap optional in ScatterPlotWithTwoHists
f679b5d 
This plot should work for both visit-level plots and coadd-level plots.
The former has no need for a skymap and the `makePlot` method here
should not require it in its function signature.
@arunkannawadi
Copy link
Member Author

generateSummaryStats requires skymap argument and cannot handle it being None. However, for the visit-level plots, addSummaryPlot had been set to False, hence not calling it in the first place. But unlike focalPlanePlot which requires camera, this plotting is generic and should not require any argument that's not absolutely necessary.

leeskelvin
leeskelvin approved these changes Apr 30, 2024
View reviewed changes
Copy link
Contributor

@leeskelvin leeskelvin left a comment

Choose a reason for hiding this comment

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

Thanks @arunkannawadi. Moving this into the kwargs seems to be optimal here.

@leeskelvin
Copy link
Contributor

PS - I would recommend a safety Jenkins with ci_hsc added into the build products prior to merge.

@arunkannawadi arunkannawadi merged commit d5ad4d2 into main Apr 30, 2024
8 checks passed
@arunkannawadi arunkannawadi deleted the tickets/DM-44085-hotfix branch April 30, 2024 22:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@leeskelvin leeskelvin leeskelvin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@arunkannawadi @leeskelvin