hipspy / hips

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

Get tile dimension from properties file #54

Closed tboch closed 7 years ago

tboch commented 7 years ago

In current version, tiles are assumed to be 512x512 pixels.This is not always the case ; for instance, the HiPS described by http://alasky.unistra.fr/PLANCK/HFI143/properties has some 256x256 pixels tiles. Here is the kind of cutout I get for this HiPS:

capture d ecran 2017-07-04 a 09 26 41

Tile dimension should be read from properties key hips_tile_width if it exists, and otherwise defaults to 512.

cdeil commented 7 years ago

I think it would be good to have tests that show that our package works for this case. @tboch - How about adding that as an example dataset to hips-extra?

adl1995 commented 7 years ago

What is the best way to handle this? By instantiating a HipsSurveyProperties object inside HipsTile and reading hips_tile_width from there? Or should the user just pass it in after reading it from the appropriate location?

tboch commented 7 years ago

@cdeil - you are right, I have added a new sample Planck-HFI143 to hips-extra It has 256x256 pixels tiles

tboch commented 7 years ago

@adl1995 - thinking again about this, I realize we can do without the properties file to get the tile dimension. Once the tile has been fetched, we just have to look at the data shape.

@cdeil - thoughts about this?

cdeil commented 7 years ago

Isn't the tile width needed already to compute the tile list at a point in time where no tiles have been fetched yet? At that point at least it would have to be accessed from HipsSurveyProperties, no?

I don't have a clear picture yet when we should take meta info from the HipsTile or HipsTileMeta or from HipsSurveyProperties. My suggestion would be to just plow on and make all the major use-cases work, and then to do a session together (or really any time) reviewing the code we have, trying to re-organise things to be as simple and clear as possible.

tboch commented 7 years ago

Well, you are right as the tile width will change the resolution computation (I hadn't thought about this first), thus the order of the tiles to download. But for now, we can just ignore this, and resolve tile width when reading the tiles data array.

cdeil commented 7 years ago

We also have tile_width as an attribute on HipsTileMeta. It's not clear to me if that is a good idea or not. The motivation to add it I think was to collect all the info / parameters needed for tile computations that don't require the data to be fetched yet. @tboch - If tile width is constant for a given HiPS survey and is always available on HipsSurveyProperties, then probably we should remove it from HipsTileMeta?

Apart from this tile width, for me also the question of where / how to store filename and URL info is still very confusing, we'll have to talk about that again. Although I think a pragmatic approach of simply implementing tile fetching and caching and seeing what works, would maybe be OK.

tboch commented 7 years ago

@cdeil - yes, tile width is constant for a given HiPS, so right, this should be stored in HipsSurveyProperties

adl1995 commented 7 years ago

@tboch Could there be a case when a survey offers multiple tile widths i.e. both 256 and 512?

tboch commented 7 years ago

@adl1995 no, that never happens, there isn't such a survey.

cdeil commented 7 years ago

@adl1995 addressed this issue in #56 . Closing it now.

@tboch - If you suggest further changes, please re-open or file a new one.