regions icon indicating copy to clipboard operation
regions copied to clipboard

Sky region plot

Open cdeil opened this issue 7 years ago • 4 comments

In #76 (I think) I argued to remove the contains and plot method on SkyRegion objects, because it can be ambiguous how to transform the sky region to a pixel region and it's better to let the user be explicit:

sky_region.to_pix(wcs).plot(...)
sky_region.to_pix(wcs).contains(...)

Now using it myself, and seeing others plot many regions in Gammapy, I find it annoying to have to do sky_region.to_pix(wcs).plot(ax=ax) all the time, and would prefer sky_region.plot(ax=ax) to work directly and do whatever DS9 does to plot, presumably already what our to_pix(wcs).plot(ax=ax) does.

@astrofrog or anyone - is that possible? Does the ax have a link to the wcs when using wcsaxes, so that we can add a one-lineer in a method SkyRegion.plot in the base class to make this work?

Another issue I have with region plot is that if I call PixRegion.plot and then plt.legend, it never shows up in the legend. Only if I call region.as_artist and ax.add_artist does it show up in the legend. Does someone know if it's possible to plot regions and have them added to the legend in a simple way? Would be nice to extend the https://astropy-regions.readthedocs.io/en/latest/plotting.html page with an example.

cdeil avatar Sep 11 '18 13:09 cdeil

This sounds like it should be possible and I like the approach you've suggested for the skyregion plot api.

The second issue, with the legend, sounds like a bug, and I don't understand it.

keflavich avatar Sep 12 '18 15:09 keflavich

@cdeil - if you are using WCSAxes, then ax.wcs should give the WCS

astrofrog avatar Sep 14 '18 11:09 astrofrog

@astrofrog - Thanks for the tip, implemented in 30eebc9 . If the current axis is a WCSAxes or one is passed in, calling plot on the sky region seems to work fine. I find this super convenient, it just does the right thing by default without having to pass the WCS all the time.

@astrofrog @keflavich @sushobhana - Please review the API / implementation.

Once it's clear what we want, I can add tests and docs.

cdeil avatar Sep 15 '18 11:09 cdeil

The approach you've taken looks fine, that's what I'd do.

keflavich avatar Sep 15 '18 16:09 keflavich