Add utils.healpix module

[adl1995]

Created a wrapper function around healpy.boundaries which returns latitude and longitude.

[coveralls]

Coverage remained the same at 100.0% when pulling e4c00b949e0ca03f06c115927256f0539cdde9ad on adl1995:utils.healpix into bb4cc7f387380adad95942ed51bd6b35cd7f1dd2 on hipspy:master.

[cdeil]

@adl1995 - Here's my comments:

• Is the implementation correct? In https://github.com/healpy/healpy/issues/393#issuecomment-305994042 Thomas mentioned that one has to call np.transpose to get the desired result?
• In any case, please add the test case from that healpy issue as an automated test here. Either in addition, or I think it would be OK to just replace the one you have here now with the other one (because we've looked at it in detail / verified the output).
• I always use numpy.testing.assert_allclose, and I'd suggest we use it also for this package unless there's a reason to use another assert function. Note that https://docs.scipy.org/doc/numpy/reference/generated/numpy.testing.assert_array_almost_equal.html also has a note at the top of the docstring suggesting not to use it, and pointing to assert_allclose as one of the good choices.
• There is this fail: https://travis-ci.org/hipspy/hips/jobs/239317653#L1006 I think it's because the lonlat option in the healpy.vec2ang(boundary_coords, lonlat=True) call was only added recently in healpy. For this reason, but even more so for speed, I would suggest that by default we always use angles in radians in this hips package, and only go to deg if needed. So in this case my suggestion would be that you simply remove the lonlat=True parameter, and adapt the test and docstring mentioning that return is in radians.
• Currently your function takes a step=1 parameter, but then you're not passing it along (unused). There's two options here, both valid in my opinion: pass the parameter along. Or omit it, and remove it's mention from the docstring. In that case, when you're not following the healpy.boundaries, you could also rename the function to healpix_corners, which is a more clear name and what we need in this package only.
• The docstring should be improved: add an intersphinx link to healpy.boundaries and healpy.vec2ang, mentioning that those are called. The parameters should be de-dented one level and the blank lines between parameters can be removed. See https://github.com/numpy/numpy/blob/master/doc/HOWTO_DOCUMENT.rst.txt or any function in Astropy or other Astropy-affiliated packages.
• I would suggest you return a tuple, not a list. I.e. instead of return [lon, lat] just put return lon, lat.
• Lastly, I would like us to use type annotations in this package and then use mypy for type checking. For an example see https://github.com/hipspy/hips/blob/master/hips/draw/simple.py#L15 where I annotated that the argument is a str and the return an int. Please also annotate this function to document what types the inputs and outputs are, and then run python -m mypy hips/utils/healpix.py to check (no output means all is good).

@adl1995 - Let me know if you have any questions, or when a new version of this PR is ready for review.

[adl1995]

I just made another commit. The modified function now returns theta, phi and I have adapted the test case accordingly. Also, the documentation was not referencing this, so I had to add automodapi:: hips.utils in docs/api.rst file. I didn't quite get what you meant by "The parameters should be de-dented one level", should the leading spaces after commas be removed?

[coveralls]

Coverage remained the same at 100.0% when pulling 9da1f1fd3e79a552839bff00fc814396dcba7920 on adl1995:utils.healpix into bb4cc7f387380adad95942ed51bd6b35cd7f1dd2 on hipspy:master.

[coveralls]

Coverage remained the same at 100.0% when pulling 9da1f1fd3e79a552839bff00fc814396dcba7920 on adl1995:utils.healpix into bb4cc7f387380adad95942ed51bd6b35cd7f1dd2 on hipspy:master.

[adl1995]

I think Travis CI build is failing at mypy. I got this as the output of python3 -m mypy hips/utils/healpix.py:

hips/utils/healpix.py:12: error: Cannot find module named 'healpy'
hips/utils/healpix.py:12: note: (Perhaps setting MYPYPATH or using the "--ignore-missing-imports" flag would help)
hips/utils/healpix.py:13: error: No library stub file for module 'numpy'
hips/utils/healpix.py:13: note: (Stub files are from https://github.com/python/typeshed)
hips/_astropy_init.py:25: error: Cannot find module named 'astropy.tests.helper'
hips/_astropy_init.py:114: error: Cannot find module named 'astropy.config.configuration'
hips/version.py:210: error: Cannot find module named 'hips._compiler'
hips/version.py:215: error: Cannot find module named 'hips.cython_version'

But, I ignored these as these were related to import errors.

[cdeil]

I think Travis CI build is failing at mypy.

No. It's failing here running pycodestyle? https://travis-ci.org/hipspy/hips/jobs/239896519#L983

python3 -m pycodestyle hips --count
hips/utils/healpix.py:19:1: W293 blank line contains whitespace

Mypy isn't running on travis-ci yet, because the issues you point out have to be fixed or silenced (something I plan to do). When discussing travis-ci, please read the logs carefully (it takes some time to understand them) and always link to lines in the logs in discussions to point me to what you looked at / comment on.

[cdeil]

I didn't quite get what you meant by "The parameters should be de-dented one level", should the leading spaces after commas be removed?

You wrote this:

    Parameters
    ----------
        nside : int

            The nside of the HEALPix map

        pix : int

            Pixel identifier

The nside parameter is indented (I think) by 8 spaces.

It should only be indented by 4 spaces, and I'd also prefer we put no blank line between parameters, i.e. it should instead be this:

    Parameters
    ----------
    nside : int
        The nside of the HEALPix map
    pix : int
        Pixel identifier

[cdeil]

Besides the points I just mentioned, I think the only comment from yesterday that isn't implemented yet is to change the test example to the one from here: https://github.com/healpy/healpy/issues/393#issuecomment-305994042

You could also put the same example in the docstring, into a section

Examples
----------

at the end. It's always nice to have a working example in the docs, no?

[adl1995]

I just added in a new section called:

Examples
--------

But, it isn't being rendered properly in the documentation. I also get this error message when building the docs:

hips/build/lib.linux-x86_64-3.4/hips/utils/healpix.py:docstring of hips.utils.boundaries:31: (SEVERE/4) Unexpected section title or transition.

[coveralls]

Coverage remained the same at 100.0% when pulling fa832132c016e7285d24abe58d1438fff27e649b on adl1995:utils.healpix into bb4cc7f387380adad95942ed51bd6b35cd7f1dd2 on hipspy:master.

[cdeil]

The processing by Sphinx of the content in an Examples section of a docstring is similar to the processing of general docs content in docs/*.rst files. You're writing restructured Text (RST) in the docstrings, with a special format for the Numpy docstring standard.

So to show code, the usual code formatting in RST works. See http://www.sphinx-doc.org/en/stable/markup/code.html

So to get the code formatted, you have to either use

>>>

to indicate that this is to be input in the Python promt. Usually that is used for little examples where you want to show input and output, such as:

>>> a = 42
>>> a + 1
43
>>> a + 42
84

For longer code examples, you usually create a "code block", either by explicitly starting it with:

.. code-block::
    from hips.utils import boundaries
    ... more code here, still indented by 4 spaces

or by starting it with this shortcut of double colon:

::
    from hips.utils import boundaries
    ... more code here, still indented by 4 spaces

or simply by writing some text and then having the code indented by four spaces:

Look at this:

    from hips.utils import boundaries
    ... more code here, still indented by 4 spaces

Understanding the docstring format is like understanding the Python code itself. You have to read the description (see https://github.com/numpy/numpy/blob/master/doc/HOWTO_DOCUMENT.rst.txt), try it out (by running through Sphinx repeatedly), and when you get stuck, ask for help. Also, often looking for a similar example in the Astropy core repo (using local search or the Github full-text search) often helps.

Try to give full working code examples that one can copy & paste into the terminal and that will run and give the output shown (if you do show the printed output in the example). I.e. in this case and generally, add the imports needed to run the example.

[cdeil]

I think the example you use here still isn't the one from the healpy issue we filed? There we use order = 3. Please change it to that exact example.

The reason I'm insisting on this is that with the current example you have here, the "reference result" is just arbitrary numbers to me, I haven't verified them. On the other hand, with the example from the healpy issue, both Thomas and I looked at the output, and compared it to the Figure from the HiPS paper, i.e. know roughly what to expect. In addition you could / should check that hovering with the mouse over the HEALPix corners in Aladin Lite or Aladin for that pixel gives roughly the coordinates you get from the code here, and then in the test leave a comment that these are "verified" numbers we're asserting against, pointing to either the healpy issue or stating that you verified it against e.g. Aladin Lite with the "show healpix grid" option and reading off the coordinates.

[coveralls]

Coverage remained the same at 100.0% when pulling 0b1359036a1436551234dd21ce47149c533c1bea on adl1995:utils.healpix into bb4cc7f387380adad95942ed51bd6b35cd7f1dd2 on hipspy:master.

[cdeil]

Looks all good now. Thanks!