hipspy / hips

Python library to handle HiPS
https://hips.readthedocs.io
13 stars 16 forks source link

Add WCSGeometry create_simple that matches Aladin Lite #39

Closed cdeil closed 7 years ago

cdeil commented 7 years ago

Currently we have this convenience methods that users can use to create a WCS: https://hips.readthedocs.io/en/latest/api/hips.utils.WCSGeometry.html#hips.utils.WCSGeometry.create It's a bit cumbersome to use (e.g. shape and crpix have different order x/y, one has to compute crpix manually before calling).

I think it would be nice to have another one that matches what Aladin Lite gives: (See http://aladin.u-strasbg.fr/AladinLite/doc/API/#init)

I.e. something like this on WCSGeometry

@classmethod
def create_simple(cls, skydir: SkyCoord, fov: Angle, shape: tuple of float):
    ... compute WCS as Aladin LIte does it, maybe calling the other create method, maybe not.
    return cls(wcs, shape)

@tboch - Could you please make a pull request, or give infos here for @adl1995 how exactly Aladin Lite computes the WCS so that he can implement it?

tboch commented 7 years ago

What Aladin Lite uses is similar to this code:

def make_wcs(ra, dec, fov_deg, width, height):
    """Create a WCS from a few params
    Parameters
    ----------
    ra : right ascension of the center
    dec : declination of the center
    fov_deg : field of view in degrees
    width : image width in pixels
    height : image height in pixels
    """"    

   resolution = float(fov_deg) / float(max(width, height))

    w = wcs.WCS(naxis=2)
    w.wcs.crpix = [width / 2., height / 2.]
    w.wcs.cdelt = numpy.array([-resolution, resolution])
    w.wcs.crval = [ra, dec]
    # this should be adapted to other type of projections
    w.wcs.ctype = ["RA---SIN", "DEC--SIN"]

    return w

Some comments:

cdeil commented 7 years ago

@tboch - Thanks!

Note that this is just an "extra constructor" as a convenience. Users still can pass any WCS to __init__ and have the full power of choosing any FITS WCS. So in my opinion we don't need to try and expose everything via these convenience constructors. But yes, where easily possible, we can add options such as rotation or projection string as you mention)

@adl1995 - Assigning to you. Please add this in a separate PR (any time after finishing up #30) and maybe consider using this simpler one in docs and tests more.

adl1995 commented 7 years ago

Can this issue be closed now?

cdeil commented 7 years ago

Yes, I think it's fully addressed.