Defining regions without centers

[larrybradley]

It would be nice to be able to define regions without specifying the center positions (or have a default center of e.g. (0,0)). This issue came up when trying to define an object-oriented interface for local background subtraction for photutils PSF fitting, where the positions of the stars would be determined in a downstream function, e.g.:

from photutils import MMMBackground, CircularAnnulus, psf_photometry
aper = CircularAnnulus(r_in=15, r_out=30)       # position of stars to be determined later
bkg = MMMBackground( sigma=3.)
psf_phot_table = psf_photometry(data, ...., background_sub=bkg, background_aperture=aper)

Inside the psf_photometry function, the apertures positions could then be defined by setting the object center attribute.

[cdeil]

@larrybradley - It looks like this is currently possible:

In [1]: from regions import CircleSkyRegion, CirclePixelRegion, PixCoord

In [2]: from astropy.coordinates import Angle, SkyCoord

In [3]: CircleSkyRegion(center=None, radius=None)
Out[3]: 
CircleSkyRegion
Center: None
Radius: None

In [4]: CirclePixelRegion(center=None, radius=None)
Out[4]: 
CirclePixelRegion
Center: None
Radius: None

But I think that's only because no-one got around to implementing input validation yet.
So if this is important behaviour, we should establish it via a unit test that contains a comment that this is on purpose.

I'm putting this on the v0.2 milestone for now.

@larrybradley - Do you have time for this in the coming weeks?

[larrybradley]

@cdeil Ah, great! Thanks.

Sure I can add a unit test to ensure this capability.

[larrybradley]

Should we make the center and radius default to None so that one can initialize a region with no arguments, e.g. CircleSkyRegion()?

[cdeil]

Ah, I see, currently it's this:

class CircleSkyRegion(SkyRegion):
    """
    A circle in sky coordinates.

    Parameters
    ----------
    center : `~astropy.coordinates.SkyCoord`
        Center position
    radius : `~astropy.units.Quantity`
        Radius in angular units
    """

    def __init__(self, center, radius, meta=None, visual=None):
        # TODO: test that center is a 0D SkyCoord
        self.center = center
        self.radius = radius
        self.meta = meta or {}
        self.visual = visual or {}

http://astropy-regions.readthedocs.io/en/latest/api/regions.CircleSkyRegion.html

For me defaults of None are OK, we're all consenting adults, right?

Although, @larrybradley , what if by accident a region falls into the hand of a child?
Maybe input validation would be better after all and for your application where some properties are only computed later, you start out by filling zeros?
:-)

@larrybradley - Either way is fine with me, feel free to make a PR with what you think is best.

[larrybradley]

Setting the defaults to (0, 0) and 0 probably makes better sense, thanks. Since these attributes are mutable, do we want to make them properties with a setter to do validation?

[cdeil]

Since these attributes are mutable, do we want to make them properties with a setter to do validation?

That would probably be the best solution, yes.

How should we implement validation?
Should we just do this?

def __init__(center, radius):
    self.center = SkyCoord(center)
    self.radius = Angle(radius)

(well, not in init, but in the setter)

I would especially like it for convenience (examples, interactive work) if I'm allowed to pass strings like radius='10 deg' which Angle understands:

>>> Angle('10 deg')
<Angle 10.0 deg>

This also works for SkyCoord

>>> SkyCoord("1h12.72m +1d12.71m")
<SkyCoord (ICRS): (ra, dec) in deg
    (18.18, 1.21183333)>

but as far as I know it's very limited, there's no way to construct e.g. a SkyCoord with a Galactic frame just from a string?

Deferring to other classes for validation would be nice to avoid duplicating work in this code, no?

For my use cases, region construction speed is not a concern, but I know that there are open issues about bad Angle and SkyCoord construction performance and maybe it's a concern for other applications?

@larrybradley - Time for a PR with a proposal how to do it for CircleSkyRegion to force some progress / feedback by others?

[astrofrog]

+1 to defaulting to a center at the origin

[cdeil]

@larrybradley - What's the conclusion of this discussion? Can you please state if there is one, and if yes, what the action items are?
Does anyone have time for this within the next ~week for the 0.2 release or should we move the milestone to 0.3?

[larrybradley]

I'm going to move this to 0.3.

[cdeil]

Moved to 0.4 milestone.

[larrybradley]

Discussion moved to #446.