Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add WCS property in HipsTile class #37

Closed
wants to merge 1 commit into from

Conversation

adl1995
Copy link
Member

@adl1995 adl1995 commented Jun 26, 2017

I added a WCS property in HipsTile class. As drawing is going to be a major part of this library, I think this is a useful addition to the class.

@cdeil
Copy link
Contributor

cdeil commented Jun 26, 2017

@adl1995 - I don't think this will work. Currently the FITS headers of HiPS tiles in FITS format don't contain a valid WCS that would be useful for drawing. See #17.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 80.252% when pulling c5cd3e6 on adl1995:tiles.wcs into 32db7fb on hipspy:master.

@adl1995
Copy link
Member Author

adl1995 commented Jun 26, 2017

But, wouldn't we still require this later on?

@cdeil
Copy link
Contributor

cdeil commented Jun 26, 2017

@adl1995 - Not necessarily, we can draw the tile without having a WCS for the tile itself.

It would be nice to have though, since if we have a WCS for the tile we can just call reproject as an alternative precise drawing method, as mentioned in #17.

There's nothing you can do here for now, I'd suggest to just close this PR without merging.
Maybe in the future, Thomas will provide code to compute the tile WCS, or maybe we won't go that way.

@adl1995 adl1995 closed this Jun 26, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants