Skip to content

Made error message more helpful when cropping to a single pixel #869

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

Merged
merged 1 commit into from
Jul 2, 2025

Conversation

ayshih
Copy link
Member

@ayshih ayshih commented Jul 1, 2025

The error message when cropping to a single pixel does not currently make it clear that one can get the single pixel by setting keepdims=True. This PR adds that helpful information. This PR also fixes a typo in the utility function's docstring.

@ayshih ayshih added the Minor Change PR only needs one approval to merge. label Jul 1, 2025
@nabobalis nabobalis added the backport 2.3 on-merge: backport to 2.3 label Jul 1, 2025
@@ -127,7 +127,7 @@ def get_crop_item_from_points(points, wcs, crop_by_values, keepdims):
Denotes whether cropping is done using high-level objects or "values",
i.e. low-level objects.
keep_dims : `bool`
keepdims : `bool`
Copy link
Member

Choose a reason for hiding this comment

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

why is keepdims, not keep_dims?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm simply making the docstring match the API.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, this was more of a question for the two maintainers of ndcube than you.

@Cadair and @DanRyanIrish, explain yourselves.

@nabobalis
Copy link
Member

Can we get a changelog please?

@ayshih
Copy link
Member Author

ayshih commented Jul 1, 2025

Changelog entry added under doc

@nabobalis nabobalis added this to the 2.4.0 milestone Jul 1, 2025
@Cadair Cadair merged commit 58c8075 into sunpy:main Jul 2, 2025
20 checks passed
meeseeksmachine pushed a commit to meeseeksmachine/ndcube that referenced this pull request Jul 2, 2025
Cadair added a commit that referenced this pull request Jul 2, 2025
…on-2.3

Backport PR #869 on branch 2.3 (Made error message more helpful when cropping to a single pixel)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.3 on-merge: backport to 2.3 Minor Change PR only needs one approval to merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants