hipspy / hips

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

Add support for RGB tiles #63

Closed adl1995 closed 7 years ago

adl1995 commented 7 years ago

As discussed in issue #62, I have added a test case of JPG tiles, which is passing. However, I haven't added a test case for PNG tiles, because the current survey only support FITS, JPG tiles.

tboch commented 7 years ago

Could you use the 2MASS6XH sample in hips-extra, which has PNG tiles?

adl1995 commented 7 years ago

@tboch The PNG tiles are already available in hips-extra. I was reluctant to add the test case as it would require a separate survey, the current one does not support PNG tiles.

I think a separate function get_test_tiles_png would server the purpose, or would it be better to take the tile path as a parameter?

tboch commented 7 years ago

I have no strong feeling about how to test this. But if the library supports PNG, we should add a test using PNG tiles.

adl1995 commented 7 years ago

I just added a test case for PNG tiles, however, it is currently failing. The pixel values sum to 0.0. I suspect this is because it doesn't fall in the sky image.

cdeil commented 7 years ago

You have committed changes to astropy_helpers: https://github.com/hipspy/hips/pull/63/files Please get rid of them (and never commit changes to astropy_helpers in the future).

cdeil commented 7 years ago

I just added a test case for PNG tiles, however, it is currently failing. The pixel values sum to 0.0. I suspect this is because it doesn't fall in the sky image.

Can you pick another survey that has PNG and data at least for part of the sky image?

tboch commented 7 years ago
 Can you pick another survey that has PNG and data at least for part of the sky image?

I'd suggest using HiPS P/AKARI/FIS/Color that has PNG and data in the galactic plane.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.2%) to 97.814% when pulling 958dc4d1dc6775004839d9dc5a01ca7465217bb2 on adl1995:rgb_support into 3cd9d60c8d1f723304d351cbdab58644e4886bbe on hipspy:master.

cdeil commented 7 years ago

@adl1995 - I merged https://github.com/hipspy/hips-extra/pull/3 just now. Does it work if you use tiles from that survey?

cdeil commented 7 years ago

@adl1995 - Maybe add a section on making an RGB color image in the high-level docs with a nice example? https://hips.readthedocs.io/en/latest/getting_started.html#more-advanced-examples

The HiPS paper uses "HiPS of the Spitzer GLIMPSE 360 survey" in Figure 4, which looks nice.

adl1995 commented 7 years ago

@cdeil I just made some updates. For the example on getting started page, should I duplicate all the steps from above or only the ones that need changing?

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.05%) to 97.613% when pulling 3c02fd09dafd027a85998ed6f130752637c04711 on adl1995:rgb_support into c9b92f1a689fa6799f624e5c710af3483395f8f6 on hipspy:master.

cdeil commented 7 years ago

For the example on getting started page, should I duplicate all the steps from above or only the ones that need changing?

As you like. I think I would re-use the geometry, but otherwise it's new.

Things to explain there:

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.05%) to 97.613% when pulling 3c02fd09dafd027a85998ed6f130752637c04711 on adl1995:rgb_support into c9b92f1a689fa6799f624e5c710af3483395f8f6 on hipspy:master.

coveralls commented 7 years ago

Coverage Status

Coverage increased (+0.05%) to 97.613% when pulling 8ca56a7036540d528fb891a811a9bec901086aa3 on adl1995:rgb_support into c9b92f1a689fa6799f624e5c710af3483395f8f6 on hipspy:master.

adl1995 commented 7 years ago

@cdeil Please merge this PR. I will make a separate PR for documentation changes.

cdeil commented 7 years ago

OK, merging now.