Photometry API proposal by astrofrog · Pull Request #2 · astrofrog/astropy-api

astrofrog · 2012-11-05T17:37:34Z

This is an initial version of the Photometry API document, based on the discussion in

https://github.com/astropy/photutils/wiki/Proposed-photutils-API

Note that for now, I have left out object detection and regions. I believe that for now we should go ahead with the idea of having aperture objects that have no specific center, because we want to be able to efficiently do photometry on tens of thousands of sources or more, and tens of thousands of instances of aperture objects is not efficient. The Region stuff could be developed separately, and just rely on the same cython routines under the hood. But I think for now we should focus on the photometry API without worrying about the regions (sorry for bringing it up in the first place!).

Note also that the standalone photometry routines can work directly with Numpy arrays as @kylebarbary was asking. They also work with NDData objects, but that doesn't prevent using simply Numpy arrays. The only place where I think we should stick with an astropy object is the Table object for the output. It really is just a structured array, so should be familiar to users, but just has a lot of nice extras.

I also tried to re-think how we deal with multiple apertures per source and different apertures for each sources, trying to make it as intuitive as possible - I'd be interested to hear what you think.

I tried to re-think the API to ensure consistency between the aperture and PSF photometry. Note that in fact, we could even consider having a high-level photometry function that could do:

results = photometry(image, (x, y), [ap1, ap2, psf1, psf2])

This would be awesome, because it would provide a way to do simultaneous aperture and PSF photometry. But that's not in the API document yet.

Things that still need to be added to this API document:

How to pass uncertainties when the data is passed just as a Numpy array
Source detection

I will work on these, but let's first see if there is agreement about what's already there.

I will not add anything about the internal API, as that is a separate matter.

Once you agree with this document, I will add you as an author, and once we've converged, I'll open a PR on the main astropy-api repository!

cc @bretonr @kbarbary @stargaser

kbarbary · 2012-11-09T18:16:24Z

Allowing a single object to represent multiple apertures has advantages. But keep in mind that it makes the classes a lot more complicated and therefore makes writing a custom Aperture class more difficult. It may also make the API for the Aperture classes less intuitive. We should think about how this would work inside the aperture_photometry function before deciding on this. An alternative is to
have

aps1 = [CircularAperture(1.), CircularAperture(2.), CircularAperture(3.)] aps1 = [CircularAperture(1.5), CircularAperture(2.5), CircularAperture(3.5)] results = aperture_photometry(image, (x, y), [aps1, aps2])

Done - I actually removed the bit about multiple apertures that are different for every source. This would make things a lot more complicated, and I don't think there's any advantage to trying to have the function do multiple apertures that are different for every source, as opposed to having the user do this manually.

kbarbary · 2012-11-09T18:51:01Z

Comments

I like the (x, y) tuple input in the photometry functions.
I will acquiesce on the Table issue: we can return Tables.
I think there should be convenience methods for aperture_photometry that allows users to not have to know about the Aperture classes if they don't need to.
The photometry functions should accept keywords such as error, mask, gain, wcs, etc. If data is an NDData object, these would override the corresponding attributes of that object. If data is an ndarray these would be optional, and provide a way to specify the uncertainty for example.

Arbitrary user statistics

We may want to have aperture_photometry accept a keyword that specifies what statistic to calculate on pixels in the aperture:

aperture_photometry(image, (x, y), [ap1, ap2, ap3], statistic=np.sum)  # sum pixels in aperture (default)
aperture_photometry(image, (x, y), [ap1, ap2, ap3], statistic=np.average)  # average of pixels in aperture
aperture_photometry(image, (x, y), [ap1, ap2, ap3], statistic=np.median)  # median of pixels in aperture

This would address astropy/photutils#8. I haven't yet figured out an elegant way to account for pixels partially in the aperture. One idea: we could force the statistic callable to accept a weights argument. In this case, a user would have to define their own median method:

def my_median(a, weights):
    return np.median(a[weights > 0.5])

# median of pixels more than halfway in aperture:
aperture_photometry(image, (x, y), [ap1, ap2, ap3], statistic=my_median)

There could also be a registry of built-in statistics, so that statistic='median' works.

stargaser · 2012-11-10T01:13:52Z

I'd really like to be able to specify aperture dimensions in arcseconds or arcminutes. Specifying it only in pixels places a burden on the user to look up the pixel size. Of course that assumes the aperture is going to applied to a higher-level object than an array.

By default, I think these should be pixels (as for x and y since not all images have WCS, but it would be easy to use other units:

from astropy import units as u a1 = CircularAperture(1.) # pixels a2 = CircularAperture(1. * u.arcsec) a3 = CircularAperture(1. * u.arcmin) a4 = CircularAperture(1. * u.degree)

does this seem like a good idea to you?

astrofrog · 2012-11-11T10:30:12Z

Thanks everyone for all the comments! I'll work on a second iteration taking everything into account.

astrofrog · 2012-11-25T22:54:16Z

@bretonr @kbarbary @stargaser - I've updated the document, taking into account all your comments. Could you review one more time and let me know whether you agree with it? I still need to add something about source detection, so if you have suggestions about this, please let me know! Also, let me know if anything else needs fixing!

kbarbary · 2012-11-26T04:13:26Z

Just note that this wouldn't quite work the way it is written. In the code for aperture_photometry(...) we have two arrays before we use the "statistic": the value of each pixel (subarr), and the fraction of the pixel in the aperture (frac). The default behavior is np.sum(subarr * frac). However, this cannot simply be replaced by np.median(subarr * frac), or even np.average(subarr * frac).

You probably realize this but it is not clear from the API document that the above is not an actual solution. Perhaps put the registry solution above and then say that a user can also define their own function that accepts values and weights, such as my_median(a, weights).

I have now clarified this.

kbarbary · 2012-11-26T16:46:46Z

What do you all think about including galaxy model photometry? (e.g., A simple Sersic profile convolved with a PSF) It seems like this is simply an extension of PSF magnitudes, in that one is fitting a model for the flux distribution of the source. However, rather than fitting a single parameter (PSF amplitude) one is fitting multiple parameters of the galaxy model.

If not defined here, it might be good to keep this sort of functionality in mind when defining the PSF photometry API, as we would want the APIs to be similar.

astrofrog · 2012-12-02T14:41:31Z

@kbarbary - good idea, I guess that there is nothing in the current API that would preclude having PSF objects have multiple parameters. The PSF object could return a list of parameters (with the default being scale) and then sub-classes of PSF could add parameters. This could fit in nicely with astropy/astropy#493. I'll add a note about this.

…multiple parameters.

astrofrog · 2012-12-02T14:57:27Z

@kbarbary - I think I've addressed all your comments - could you give it one final read before I open it for review by the other developers?

@bretonr @stargaser - do you have any other comments?

bretonr · 2012-12-03T23:09:27Z

Hey guys

I'm really sorry I've been traveling outside the country with poor
connectivity. Will check things out asap. In any case please move forward,
I trust you ;-)

Rene

Sent from my iPhone

On 2012-12-02, at 3:57 PM, Thomas Robitaille notifications@github.com
wrote:

@kbarbary https://github.com/kbarbary - I think I've addressed all your
comments - could you give it one final read before I open it for review by
the other developers?

@bretonr https://github.com/bretonr
@stargaserhttps://github.com/stargaser- do you have any other
comments?

—
Reply to this email directly or view it on
GitHubhttps://github.com//pull/2#issuecomment-10930465.

stargaser · 2012-12-04T23:37:46Z

I like the capability to make a custom statistic. However it reminded me that when photometry of a calibrated NDData object is run, meaning there are units of Jy/pixel or MJy/sr or whatever, it's nice to get units of Jy or whatever out in the output table. Perhaps this should be done by a later "aperture correction" stage.

Automatic unit conversion would indeed be nice. I'll add a note to the document to mention that this would be nice, and it should just be part of the internal magic (like coordinate transformations).

stargaser · 2012-12-04T23:39:00Z

Looks good, I just made one comment on getting calibrated brightnesses in the output table, which perhaps has to be handled by an "aperture correction" task.

astrofrog · 2012-12-05T08:27:24Z

@stargaser - thanks for reviewing this! Since you've all agreed to this document now, I will merge this PR, and then open a PR to the main astropy-api repository.

Photometry API proposal

astrofrog · 2012-12-05T08:40:01Z

@stargaser @bretonr @kbarbary - merged into my photometry-api branch. Thanks for all your feedback! I will now prepare a PR and email to the main list for feedback :-)

cdeil · 2014-04-27T16:50:35Z

Hi @sophiathl !

I'm trying to get an overview of photutils-related codes by collecting links here.
Is this (https://github.com/durga2112/imagecube) the imagecube package you mentioned last year?

sophiathl · 2014-04-27T17:32:50Z

Hi Christoph!

Sounds great, yes, that's the link to imagecube!

On Sun, Apr 27, 2014 at 12:50 PM, Christoph Deil
notifications@github.comwrote:

Hi @sophiathl https://github.com/sophiathl !

I'm trying to get an overview of photutils-related codes by collecting
links here https://github.com/astropy/photutils/wiki/Links.
Is this (https://github.com/durga2112/imagecube) the imagecube package
you mentioned last year?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/2#issuecomment-41501752
.

astrofrog added 4 commits November 5, 2012 13:52

Initial draft of photometry API document

9aaa86a

More improvements to API document

59e0790

Added API for multiple apertures

ebf50f4

typos

4a42a48

kbarbary reviewed Nov 9, 2012
View reviewed changes

stargaser reviewed Nov 10, 2012
View reviewed changes

kbarbary reviewed Nov 26, 2012
View reviewed changes

astrofrog added 8 commits December 2, 2012 15:55

Added example with units

c3a3b6c

Added more details about PSFs

01ff470

Adressed initial comments

702871a

Added a section about wcs/mask/uncertainty without NDData

a4a2503

Minor fixes

6cc74af

Clarify custom statistic for aperture photometry

408cf72

Added section of varying apertures/PSFs

17fc872

Simplify multiple aperture API, and added note about PSF models with …

8b2e535

…multiple parameters.

stargaser reviewed Dec 4, 2012
View reviewed changes

Added a note about unit conversion for flux

b79ed3a

astrofrog added 3 commits December 5, 2012 09:30

Added a requirement

99b1a12

Minor typo

a887c59

PEP8

119715c

astrofrog added a commit that referenced this pull request Dec 5, 2012

Merge pull request #2 from astrofrog/photometry-api-proposal

dd78f3b

Photometry API proposal

astrofrog merged commit dd78f3b into photometry-api Dec 5, 2012

Conversation

astrofrog commented Nov 5, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kbarbary commented Nov 9, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astrofrog commented Nov 11, 2012

Uh oh!

astrofrog commented Nov 25, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

kbarbary commented Nov 26, 2012

Uh oh!

astrofrog commented Dec 2, 2012

Uh oh!

astrofrog commented Dec 2, 2012

Uh oh!

bretonr commented Dec 3, 2012

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stargaser commented Dec 4, 2012

Uh oh!

astrofrog commented Dec 5, 2012

Uh oh!

astrofrog commented Dec 5, 2012

Uh oh!

cdeil commented Apr 27, 2014

Uh oh!

sophiathl commented Apr 27, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants