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 healpix to hips conversion method #123

Merged
merged 7 commits into from
Jul 4, 2018

Conversation

adonath
Copy link
Collaborator

@adonath adonath commented Jul 4, 2018

This PR adds the healpix to hips conversion functionality. It is exposed via the method healpix_to_hips.
I added a simple test using a healpix image filled with np.arange() in the nested scheme and compared the sum of the indices to manually created reference values. I also added an environment.yml defining a hips-dev environment.

@cdeil cdeil self-assigned this Jul 4, 2018
@cdeil cdeil added this to the 0.3 milestone Jul 4, 2018
@coveralls
Copy link

coveralls commented Jul 4, 2018

Coverage Status

Coverage increased (+0.2%) to 96.301% when pulling 674e3e6 on adonath:healpix_to_hips_conversion into 8d6078f on hipspy:master.

Copy link
Contributor

@cdeil cdeil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adonath - This is great, thanks!

A high-level docs page with one example would be great.
I also think a unit test for healpix_to_hips_tile asserting on one or two pixel values would be good to have.

Let me know if you have time to apply these suggestions, or if I should do it tomorrow.

filename.parent.mkdir(exist_ok=True, parents=True)
tile.write(filename=filename)

# Write a minimal property file
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this class it a dict representation of the properties file?
https://hips.readthedocs.io/en/latest/api/hips.HipsSurveyProperties.html
Can you use it here and add the to_string and write there if it doesn't exit already?

Either way I'd suggest to use this for the write text:
https://docs.python.org/3/library/pathlib.html#pathlib.Path.write_text
It also closes the file on the end of the method call, and is one line less and doesn't need the with and local variable f, i.e. ist just a bit simpler overall.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@adonath
Copy link
Collaborator Author

adonath commented Jul 4, 2018

@cdeil OK to add the high level docs example later, once the support for color image is added?

@cdeil
Copy link
Contributor

cdeil commented Jul 4, 2018

Sure

@cdeil
Copy link
Contributor

cdeil commented Jul 4, 2018

@adonath - Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants