Skip to content

Fix: unexpected behavior in Cutout2D.plot_on_original#19829

Merged
pllim merged 3 commits into
astropy:mainfrom
juanep97:plot_on_original
May 29, 2026
Merged

Fix: unexpected behavior in Cutout2D.plot_on_original#19829
pllim merged 3 commits into
astropy:mainfrom
juanep97:plot_on_original

Conversation

@juanep97
Copy link
Copy Markdown
Contributor

@juanep97 juanep97 commented May 28, 2026

Description

This pull request is to address unexpected behavior in Cutout2D.plot_on_original (see Issue #19828). Current implementation spans pixels that are not included in the cutout array. The proposed change ensures the plotted region correctly covers the included pixels, completely.

Fixes #19828

@github-actions
Copy link
Copy Markdown
Contributor

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see instructions for rebase and squash.
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@pllim pllim added the Bug label May 28, 2026
@pllim pllim added this to the v7.2.1 milestone May 28, 2026
@pllim pllim added backport-v7.2.x on-merge: backport to v7.2.x backport-v8.0.x on-merge: backport to v8.0.x labels May 28, 2026
@pllim
Copy link
Copy Markdown
Member

pllim commented May 28, 2026

Nothing broke before/after this patch without changes to existing test. Is this functionality not tested in CI? 🤔

Need a change log. Thanks!

@pllim pllim requested review from bmorris3 and mwcraig May 28, 2026 19:25
@juanep97 juanep97 changed the title fix unexpected behavior in Cutout2D.plot_on_original Fix: unexpected behavior in Cutout2D.plot_on_original May 29, 2026
Copy link
Copy Markdown
Member

@mwcraig mwcraig left a comment

Choose a reason for hiding this comment

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

Thanks again for the detailed bug report and the quick fix @juanep97.

This looks good to me -- I don't see a great way to test this. The easiest would probably to get the patch object from the axes and check its location.

I have confirmed locally that the code here fixes the issue in the bug report.

I'll defer to you on the need for a test @pllim

@larrybradley -- it would be great if you could take a quick look at this.

@mwcraig mwcraig requested a review from larrybradley May 29, 2026 14:53
@pllim
Copy link
Copy Markdown
Member

pllim commented May 29, 2026

Larry is also visualization maintainer and might have a good idea on how this can be tested, so I defer to him as well. 😅

Copy link
Copy Markdown
Member

@larrybradley larrybradley left a comment

Choose a reason for hiding this comment

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

Thanks, @juanep97 ! I pushed a regression test for this fix.

@pllim pllim merged commit 2ef8fbb into astropy:main May 29, 2026
38 checks passed
@pllim
Copy link
Copy Markdown
Member

pllim commented May 29, 2026

Thanks, all! Great teamwork.

pllim added a commit that referenced this pull request May 29, 2026
…829-on-v7.2.x

Backport PR #19829 on branch v7.2.x (Fix: unexpected behavior in Cutout2D.plot_on_original)
@juanep97 juanep97 deleted the plot_on_original branch May 29, 2026 23:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport-v7.2.x on-merge: backport to v7.2.x backport-v8.0.x on-merge: backport to v8.0.x Bug nddata

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Cutout2D.plot_on_original does not match the pixels in the cutout array

4 participants