Skip to content

Fix modeling separability of pseudocylindrical, Bonne and HEALPix projections - #20590

Closed
Thanatos9404 wants to merge 1 commit into
astropy:mainfrom
Thanatos9404:fix-projection-separability
Closed

Thanatos9404 wants to merge 1 commit into
astropy:mainfrom
Thanatos9404:fix-projection-separability

Conversation

@Thanatos9404

Copy link
Copy Markdown

Description

This pull request is to address separability_matrix and is_separable treating the pseudocylindrical (SFL, PAR, MOL, AIT), Bonne (BON) and HEALPix (HPX) projections as separable. These classes set _separable = True, but their first output depends on both inputs because the meridians are curved.

SFL, PAR, MOL and HPX now return [[True, True], [False, True]] from _calculate_separability_matrix, since their second output depends only on the second input (this also holds in the HPX polar facets for any H and X). AIT and BON, whose second output also depends on the first input, now report a full matrix. For BON with the default theta1=0, which wcslib evaluates as SFL, the full matrix is a conservative over-report.

To check for other cases, I perturbed each input of every Pix2Sky_*/Sky2Pix_* class and compared which outputs change against separability_matrix; after this change none of them under-reports.

Fixes #20578

AI Disclosure

I used Claude Opus 5.5 (Anthropic) through Claude Code to investigate the issue, draft the code change, tests and this description, and run the test suites. I reviewed all of it and can explain every change.

  • I certify that I am human and that I take full responsibility for this pull request including all interactions with reviewers.

Merge method

  • By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

These projections set ``_separable = True``, so ``separability_matrix``
reported their two outputs as independent although the first output
depends on latitude.  SFL, PAR, MOL and HPX now report a triangular
matrix (the second output depends only on the second input), while AIT
and BON report a full matrix.
@Thanatos9404
Thanatos9404 requested a review from a team as a code owner October 12, 2026 12:16
@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.

@giles

giles Bot commented Oct 12, 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 13 26
Issues opened 1 1

@giles giles Bot closed this Oct 12, 2026
Thanatos9404 added a commit to Thanatos9404/astropy that referenced this pull request Oct 12, 2026
@Thanatos9404

Copy link
Copy Markdown
Author

Hi maintainers, thanks for explaining the new-contributor process!

I investigated #20578 as a correctness issue rather than because it was blocking one of my own projects. The existing separability matrices incorrectly treat certain sky projection outputs as independent, even when they mathematically depend on both inputs.

For example, in the Sanson-Flamsteed projection, x = longitude × cos(latitude), so the first output depends on both longitude and latitude. However, the existing separability matrix does not reflect that dependency. This can also lead to incorrect axis-correlation information in downstream WCS workflows.

My PR addresses this for the affected pseudocylindrical, Bonne, and HEALPix projections. I also added parameterized regression tests covering both transformation directions and different projection configurations.

I've disclosed my use of Claude Code in the PR description, and I understand that I'm personally responsible for the implementation and any changes requested during review.

I've also joined the Astropy Slack workspace and will introduce myself in #new-contributor-requests.

I'd appreciate the opportunity to have the PR reviewed and am happy to discuss the technical details or make further improvements.

Thanks for your time!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

separability_matrix reports pseudocylindrical, Bonne and HEALPix projections as separable

2 participants