search

hipspy / hips

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

Improve tile and allsky orientation handling #90

Closed cdeil closed 7 years ago

cdeil commented 7 years ago

This PR improves the all-sky image implementation and tests to also work with FITS images.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.01%) to 96.699% when pulling 40aaaaf0e7b242e4d02c8714d46abe60b69ebd5f on cdeil:allsky2 into fce110c0e2bf31baba5d9050ccf08a9c67493dff on hipspy:master.

cdeil commented 7 years ago

I am working towards correct HEALPix <-> HiPS tile pixel mapping, so that we can also generate HiPS, not just read and draw it. What I found is that the up-down flip between FITS relative to {PNG/JPEG} is there both for tiles and all-sky images: https://github.com/cdeil/hipsperiments/tree/master/tile_orientation

One can see that all_sky.tile(ipix) doesn't return the same tile for FITS and JPEG, i.e. it's currently broken for one of the two, and I'm not even sure which one.

I will implement a function that implements HEALPIX ipix to Hips tile (x, y) pixel coordinate mapping next, and that is yet another case where the up / down flip needs to be taken into account.

My current thinking is to move the up / down flip from https://github.com/hipspy/hips/blob/fc5a870bf4d8215a8818d3068749ea97e7029932/hips/draw/simple.py#L175 to a central location where it will be applied correctly for all cases where it needs to be applied. Maybe it could be part of HipsTile.data (and the corresponding reverse from_numpy) that at that point we call numpy.flipup on the data array? Which tile orientation should we make the "standard" one in our package? I.e. should we always convert to the FITS orientation or the JPEG orientation?

@adl1995 @tboch - Please comment if you have any thoughts on this.

If I don't hear back, I'll just go ahead and implement a solution I like in this PR and we can then still discuss and change if needed in the coming days / weeks. I'm leaning towards always converting to the FITS orientation internally, and to always do it at the point where we to to and from numpy array with the data.

adl1995 commented 7 years ago

@cdeil I would also prefer if we convert to FITS orientation by default.

cdeil commented 7 years ago

I've made the change to flipping JPG and PNG tiles and all-sky images to the FITS convention in to_numpy and from_numpy in 26b98de in this PR.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.2%) to 96.947% when pulling 26b98de91ae30da35957bbec43b7ce109dc7b109 on cdeil:allsky2 into 1b36234b5254021e4be72333d28d50fc91097130 on hipspy:master.

cdeil commented 7 years ago

I was really struggling with the orientation of tiles and the whole image for the all-sky images. I think with acd58c882d538d4255239c3405356602dc1906d6 now results are correct.

I'm merging this now and will continue with HEALPIX ipix <-> tile (x_idx, y_idx) mapping in a separate future PR.