API question: to_patch or Artist or...?

[keflavich]

What API should we adopt for plotting region objects?

Options we have considered:

• region.to_patch()/region.to_patch_collection()
• region.Artist
• region.Patch/PatchCollection

Which do you prefer? cc @joleroi, @astrofrog @cdeil

[cdeil]

The photutils apertures have a .plot(ax=None) method which creates the patch and calls ax.add_patch(patch):
https://github.com/astropy/photutils/blob/master/photutils/aperture_core.py#L500

I can see how splitting the two steps in separate methods is more modular / a good thing.

But to form an opinion on which of the options you propose are better, I'd have to see usage examples ... whichever is nicer to use will be my preference.

@bsipocz and @larrybradley -- You implemented the aperture plot methods in photutils. What do you think?
(FYI: I think the plan discussed / proposed at PyAstro16 is to move all the aperture code over to here, call it regions, add features and in a few months put it in the core as astropy.region.)

[keflavich]

I like having a plot(ax=None) method, possibly in addition to a get_patch method. Is that too much?

[bsipocz]

There are a few issues with the photutils .plot method I never managed to iron out, but maybe this would be time for that, I just need to find my notes on it.

Also, do you mean to move over only the geometry part of the apertures, or everything including the do_photometry method? I totally support the first option, but would the latter be useful as well?

[larrybradley]

If the patches are useful by themselves (are they?), then I agree with @keflavich: a get_patch() method to return the patch and a plot() method that internally calls get_patch() and then plots it. In other words, I think plotting an aperture should remain one plot() call and not require calling get_patch() followed by plot(), or plot(get_patch()), etc.

[larrybradley]

@cdeil Are there notes from the conference about these region plans?

[larrybradley]

Instead of get_patch() perhaps it should be called make_patch()?

[cdeil]

+1 to make_patch.
We can still discuss if we want to hide it as _make_patch later, in case someone feels there's no use-case for it for end-users to call this.

[cdeil]

@larrybradley - Here's the notes from the regions session at PyAstro16:
https://docs.google.com/document/d/17owovjcpNFirz-f8qCWWxvcHHPGGpO5JJic5WbtD9JY/edit

@joleroi will make a PR soon updating the README and docs/index in this repo to reflect the plan.

My understanding is that this is the plan:

• The existing pyregion is deprecated. The functionality there is re-built here.
• The functionality from photutils.aperture is integrated into the region classes here.
• We let people use this regions package for a few weeks or months as a separate package
• Then we merge it into the core as astropy.regions
• Once things have stabilised, we can replace the aperture photometry code in photutils to use astropy.regions.

@larrybradley Concerns? Sounds good?

[larrybradley]

Thanks, @cdeil. Sounds good to me.

[astrofrog]

Sorry for commenting late on this, but we could use as_patch instead of make_patch, since that would read better in code (region.as_patch). If we think there might be other plotting packages in future, we should maybe call it region.as_mpl_patch although I could see that also being a keyword option in future.

[cdeil]

I see at http://astropy-regions.readthedocs.io/en/latest/api.html that we now have as_patch on the SkyRegion and PixelRegion base classes, and as an example it's implemented for circles.

@keflavich and all - Can we close this issue?
Or alternatively --- what's the remaining action item?
Should the abstract method be moved to the base Region class, or should it remain on SkyRegion and PixelRegion only?

[cdeil]

I think this is done, closing this now.

If someone still have a thought on the question from my last comment, please reply here or make a PR if you think something should be changes.

Should the abstract method be moved to the base Region class, or should it remain on SkyRegion and PixelRegion only?