Skip to content

Postage stamps from WCS #3594

Closed
Closed

Description

@gully

I see a good amount of duplicate code on GitHub for making "Postage Stamps" from a larger file. A GitHub search for "astropy postage stamp WCS" provides 15 hits. A search for "postage stamp" within the astropy repository yielded nothing, so I think it's safe to say that this functionality does not exist specifically in astropy. Am I wrong? Making "postage stamps" is a common use case in astronomy.

I think it would make sense to add this functionality directly into astropy, probably in the wcs module. I know it's lame to make a feature request rather than a pull request, but when it comes to wcs there is a lot of nuance that is probably over my head. Like I said, there are many existing examples of this functionality, so those would be a great starting point. I would be happy to collaborate with someone on this feature, if it is deemed worthy of integration into astropy.

Activity

  1. astrofrog commented on Mar 13, 2015

    @astrofrog
    Member

    Just a quick note - maybe this can just be a simple mash-up of astropy.coordinates, astropy.wcs and WCSAxes.

  2. gully commented on Mar 13, 2015

    @gully
    Author

    Just a thought- this might be better suited for APLpy too

  3. embray commented on Mar 13, 2015

    @embray
    Member

    Yeah I have a feeling this kind of specific application doesn't belong in the Astropy core, but I'm willing to be wrong on that.

  4. patti commented on Mar 25, 2015

    @patti
    Contributor

    I agree. I expected to find a function to do this in Astropy- as it seems many other have- before writing my own. I don't see why it wouldn't belong given that it's just a straightforward application of the existing functionality. The actual plotting can be left up to the user if that's the concern. I'll work on this.

  5. astrofrog commented on Mar 25, 2015

    @astrofrog
    Member

    I wonder if this should maybe live in nddata and be a utility function that would make use of extract_array and basically take an array, WCS, SkyCoord, and then return arrays returned by extract_arrays?

    The plotting can indeed be left to the user as suggested by @patti, but the function to go from (data, wcs, coordinates) -> (subarrays, subwcs) should live in astropy core.

    @patti - feel free to give it a shot!

  6. cdeil commented on Mar 25, 2015

    @cdeil
    Member

    What exactly is a postage stamp?

    Is it something like (or exactly) extract_array or the proposed crop function here:
    astropy/imageutils#4

    Note that if this postage stamp function should use extract_array it might not be a good idea to start this PR now, as long as #3333 is still open. (I haven't followed the discussion, but it seems to be an endless discussion because different people have different opinions what this function should do exactly).

    I'm all for having nice functions to cut out a series of images for a specified list of catalog positions and width / height and maybe even to plot them on a grid of panels in astropy core ... it just seems a bit hard to get consensus how to implement this and get something merged, so @patti , this is my warning that it might be a little frustrating if this is your first PR for Astropy and then you have to re-write it five times before it's merged.

  7. astrofrog commented on Mar 25, 2015

    @astrofrog
    Member

    Right, there are two ingredients here - the ability to extract cutouts, and making actual postage stamps plots. I think that what astropy should provide is definitely the former, and maybe in the future the latter.

    @cdeil is right that it might be worth waiting a few days until extract_array is fixed before attacking this, but then I think it would be handy to have a cutout/crop function in the core. While we can go more complicated as suggested in astropy/imageutils#4 and have bounding boxes etc. I think there is definitely room for a function that just takes SkyCoord and radius since it's a common case.

  8. patti commented on Mar 25, 2015

    @patti
    Contributor

    I haven't used extract_array, but the issue is relevant so it's probably worth holding off. The way I've done this is just to take the RA, Dec position and some angular width as input to get the angular bounds and then use WCS.all_world2pix to get the pixel bounds of the sub-array. Nothing too fancy. Return the subarray, pixel bounds, and perhaps RA,Dec maps.

  9. astrofrog commented on Mar 25, 2015

    @astrofrog
    Member

    @patti - as a side note, there are two functions in astropy.wcs.utils that can help with this, which are skycoord_to_pixel and pixel_to_skycoord:

    https://github.com/astropy/astropy/blob/master/astropy/wcs/utils.py

    These will automatically convert the coordinates to the right frame for the WCS (e.g. Galactic coordinates will be converted to FK5 if WCS is in FK5). This could help generalize the cutout code.

  10. embray commented on Mar 25, 2015

    @embray
    Member

    👍 Sounds good so far.

  11. pllim commented on Mar 25, 2015

    @pllim
    Member

    Sounds like a nice addition to imutils too. What do you think, @larrybradley ?

  12. larrybradley commented on Mar 25, 2015

    @larrybradley
    Member

    Actually, the Cutout class that I wrote for imutils (https://github.com/spacetelescope/imutils/blob/master/imutils/core.py#L333) already handles WCS, at least in the sense that the central position can be a SkyCoord (which is converted to pixel space with skycoord_to_pixel). The cutout shape is always in pixels though (one could have an angular size, but that becomes trickier if there are geometric distortions). I haven't yet updated Cutout to return an updated WCS for the cutout data, but will soon. That essentially requires shifting the CRPIX_i to keep the tangent point the same (which is also the reference point for geometric distortion corrections).

    If there is a desire to put this in the Astropy core (and it seems like there is), I'll submit a PR with my Cutout class.

  13. larrybradley commented on Mar 25, 2015

    @larrybradley
    Member

    I should mention that I too want to use extract_array in Cutout when #3333 is merged.

  14. gully commented on Mar 25, 2015

    @gully
    Author

    👍

  15. eteq commented on May 29, 2015

    @eteq
    Member

    @larrybradley - so are you planning to extract Cutout into the core? It sounds from this thread like that's much desired, so I think it makes sense to create a PR to do that.

  16. astrofrog commented on May 31, 2015

    @astrofrog
    Member

    +1!

  17. larrybradley commented on Jun 5, 2015

    @larrybradley
    Member

    I've created a PR for Cutout in #3823. I still need to add tests though.

  18. mcara commented on Jun 11, 2015

    @mcara
    Contributor

    I would like to bring to your consideration the following issues:

    1. With the current approach the new WCS' CRPIX value may be outside the "stamp" (I hate this term) image. In itself this is not a dangerous thing or at least I cannot think of anything bad about it. I just want to bring this to your attention and maybe someone will have some information that I am not aware of.
    2. If you decide to keep CRPIX inside the the image, then SIP coefficients will have to be recomputed.
    3. This code has to remove any reference to tabular distortions (e.g. det2im) as the code does not take care to recompute the tabulated distortions. This is true regardless of the approach used to recompute CRPIX. Also, this may require modification of type of WCS.
  19. astrofrog commented on Jun 11, 2015

    @astrofrog
    Member

    @mcara - I think it makes sense to keep CRPIX be the same world position, because it's not just the distortions that need to be recomputed but the spherical projection will be incorrect so the image would have to be reprojected (except in the rare case of the -CAR projection).

  20. larrybradley commented on Jun 11, 2015

    @larrybradley
    Member

    @mcara I don't think it's a problem to have CRPIX outside of the cutout image. The advantage of shifting CRPIX (and not CRVAL) is that the existing SIP coefficients will work fine and the spherical projection remains the same (as @astrofrog mentioned).

    Good point on the tabular distortions. Unfortunately, I don't think there's any good way to propagate them and apply them to the cutout image (thankfully these distortions are usually small). If they are present, then I'll issue a warning and remove them from the cutout wcs object. Otherwise, the tabular distortions would be applied incorrectly (wrong pixels) to the cutout image. Thanks.

  21. cdeil commented on Aug 4, 2015

    @cdeil
    Member

    @larrybradley's Cutout2D class pull request #3823 was merged. The high-level docs are here.

    What about this feature request issue? Can it be closed? Or is there something left to do?

  22. mwcraig commented on Aug 4, 2015

    @mwcraig
    Member

    Closing, since as @cdeil points out the functionality is in #3823 (which, in the PR comments, mentions this issue but didn't use the magic word "closes" :))

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions