search

cds-astro / ipyaladin

An IPython Widget for Aladin Lite, the sky viewer.
https://cds-astro.github.io/ipyaladin/
BSD 3-Clause "New" or "Revised" License
126 stars 26 forks source link

Merge astropy region support to Master #88

Closed Xen0Xys closed 4 months ago

Xen0Xys commented 5 months ago

Fixes #16 Fixes #87

Currently supported :

ManonMarchand commented 5 months ago

Maybe this should have a whole new example file? Or be included in the stc-s one (that would be renamed into something similar to adding shapes)?

Xen0Xys commented 5 months ago

Already done, I changed the content and the name of the example 9 to add all implemented shapes examples.

bmatthieu3 commented 5 months ago

From discussion with the others, I think that it has no sense accepting PixelRegion because it depends on a WCS. The user still can convert a PixelRegion to a SkyRegion using the to_sky method by providing a wcs. So I would drop the support of all the Pixel regions

ManonMarchand commented 5 months ago

Can you add a changelog ?

fxpineau commented 5 months ago

@ManonMarchand @bmatthieu3 If the support of astropy regions is added here (in ipyaladin), what about reusing (a part of?) the code to also support astropy regions in MOCPy?

ManonMarchand commented 5 months ago

@ManonMarchand @bmatthieu3 If the support of astropy regions is added here (in ipyaladin), what about reusing (a part of?) the code to also support astropy regions in MOCPy?

Yes, that'd be super nice too. Let me reference the corresponding mocpy issue https://github.com/cds-astro/mocpy/issues/103 so that we can read this later when needed.