Skip to content

Return a scalar from PolygonPixelRegion.contains for a scalar PixCoord - #722

Closed
syahra712 wants to merge 2 commits into
astropy:mainfrom
syahra712:fix-polygon-contains-scalar
Closed

syahra712 wants to merge 2 commits into
astropy:mainfrom
syahra712:fix-polygon-contains-scalar

Conversation

@syahra712

@syahra712 syahra712 commented Oct 22, 2026 •

Copy link
Copy Markdown

PixelRegion.contains / covers are documented to return a bool for a scalar position. PolygonPixelRegion (and RegularPolygonPixelRegion) instead return a shape (1,) array, because _containment wraps the input in np.atleast_1d:

>>> poly = PolygonPixelRegion(PixCoord([40, 60, 55], [30, 35, 55]))
>>> poly.contains(PixCoord(50, 40))
array([ True])          # every other pixel shape gives np.True_

This also leaks into compound regions built with a polygon, and into anything that checks np.ndim or indexes the result.

Fix: keep the input shape (ravel for the Cython helpers, then reshape back) and index with [()], so a scalar input gives a scalar while 1-D and 2-D inputs are unchanged.

Test: the shared BaseTestPixelRegion.test_contains_scalar passed before only because it used assert reg.contains(p), and a one-element array is truthy. It now also asserts np.ndim(...) == 0, so the check covers every pixel shape. On main it fails for the two polygon classes. With this change the full test suite passes (2146 passed) and flake8 is clean.

Added a CHANGES.rst entry under 0.14 Bug Fixes.

AI-assisted (Claude): I used it to help read the code and draft the test. I reviewed and ran everything myself.

_containment wrapped the input in np.atleast_1d, so a scalar PixCoord
gave a shape (1,) array instead of a bool like every other shape. The
input shape is now kept. The shared scalar test checks the result is
0-d for every pixel region.
@giles

giles Bot commented Oct 22, 2026

Copy link
Copy Markdown

Hi 👋 and thank you for your contribution! 🙏

This pull request is being closed automatically - but don't worry, this is not the end, and we may re-open it.

For some background, we have recently started seeing a rapid increase in the number of pull requests opened. Some of these are from good-faith humans, but some of which are from autonomous LLM agents or humans using LLMs who do not have a genuine understanding of, or interest in, the project. We are therefore auto-closing pull requests from new contributors, but if you are a good-faith human, we want to make sure your pull request gets considered! So if you would like us to re-open your pull request so that it gets reviewed, you need to do two things:

  1. Add a comment here to explain why you need the changes here to be considered, as in how the bug or missing feature affects your work, or whether this is an issue you have encountered but does not affect you.

  2. Join the astropy slack using this invite link and head over to the #new-contributor-requests channel to introduce yourself and let us know about this pull request

The second step is important, as we may otherwise miss notifications about this pull request.

To be clear, our AI policy does allow the use of LLMs as part of contributions, but we need to see authentic engagement and understanding from humans making the contribution.

Notes for maintainers

If you know this is a genuine user and the PR is is compliance with the AI policy, you may re-open this straight away. After two successful merged pull requests, the user will be added to the allowlist and future PRs will no longer be closed automatically.

This user has not made any pull requests to this repository prior to this one.

In addition, here are some statistics on the user's activity on GitHub:

Last 24 hours Last 7 days
Pull requests opened 7 0
Issues opened 0 8

@giles giles Bot closed this Oct 22, 2026
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.

1 participant