Skip to content

Implement CircleSectorPixelRegion#486

Draft
adonath wants to merge 7 commits into
astropy:mainfrom
adonath:circle_sector_pix_region
Draft

Implement CircleSectorPixelRegion#486
adonath wants to merge 7 commits into
astropy:mainfrom
adonath:circle_sector_pix_region

Conversation

@adonath

@adonath adonath commented Oct 27, 2022

Copy link
Copy Markdown
Member

This PR is a draft to implement a CircleSectorPixelRegion. I needed this for a specific analysis, so I decided to just implement it. Here is a notebook to illustrate that the bounding box and contains methods work correctly for all quadrants https://github.com/adonath/notebooks-public/blob/master/circle_sector_pix_region_illustration.ipynb

Some ToDos:

  • Implement a CircleSectorSkyRegion
  • Implement CircleSectorPixelRegion.to_sky()
  • Implement CircleSectorPixelRegion.to_mask()

@adonath adonath changed the title Implement CircleSectorPixRegion Implement CircleSectorPixelRegion Oct 27, 2022
@adonath

adonath commented Aug 1, 2024

Copy link
Copy Markdown
Member Author

@larrybradley is this a desired feature? Are there any guidelines with respect to adding new shapes to regions?

@larrybradley

Copy link
Copy Markdown
Member

@adonath This looks like a nice addition to me! Implementing the "exact" mode for exact partial pixel overlaps could be tricky.

@jzuhone

jzuhone commented Apr 6, 2025

Copy link
Copy Markdown

I'd love to help with this. What do we need to get it across the finish line?

@keflavich

Copy link
Copy Markdown
Contributor

@adonath are you still working on this? If the example works, maybe we can just merge (after resolving conflicts) and leave the unimplemented modes as open issues?

@adonath

adonath commented Apr 7, 2025

Copy link
Copy Markdown
Member Author

@jzuhone @keflavich I have not made any progress since, but I would be happy rebase and open issues for the remaining tasks.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants