astropy-healpix
astropy-healpix copied to clipboard
High-level API; Remove HEALPix class?
Currently we offer the same functionality via three interfaces (see API).
- Functions that use Numpy arrays or Quantity for angles. E.g.
lonlat_to_healpix(lon, lat, nside, return_offsets=False, order='ring')
- A HEALPix class with
(nside, order, frame)
data members. One-line methods re-expose functions. - Functions in
astropy_healpix.healpy
. Drop-in replacements for healpy. To help transition code and with internal testing.
I would like us to discuss and make a decision next week at the workshop on the following questions:
- Remove the HEALPix class?
- Hide
astropy.healpix.healpy
from the docs?
Remove HEALPix class?
The HEALPix class (see high_level.py contains nside
, order
and frame
properties, as well as ~ 10 one-line methods re-exposing the function API.
The benefit of the class is that it's very nice for users (see docs):
>>> from astropy_healpix import HEALPix
>>> hp = HEALPix(nside=16, order='nested')
>>> lon, lat = hp.healpix_to_lonlat([1, 442, 2200])
>>> hp.boundaries_lonlat([120], step=1)
A single-class API is nice to import and remember, and only having to pass some parameters once instead of to multiple function calls is convenient.
However, offering the class also has drawbacks:
- The usage of
nuniq
indexing is becoming more common, and I think we should support arrays for nside (see #66 #91). This makes it awkward to passnside
ininit
, it should really be moved to the method calls, leaving onlyorder
andframe
ininit
. - This also means there is no efficiency benefit of the class, i.e. to pre-compute something for a given
nside
. We could of course only support scalarnside
in the class, and only allow arraynside
in the functions, like @lpsinger did in #71 . - The zen of Python says "there should be one way to do it", and adding the class is just offering a second API to do the same thing, splitting the docs and user base.
I'm +1 to remove the class, and am volunteering to make a PR removing it and rewriting the docs to use the functions.
Hide astropy.healpix.healpy
from the docs?
Another question that we should decide before making a PR for astropy.healpix
for the Astropy core repo is whether astropy.healpix.healpy
should be shown in the public API and docs. At the moment it appears like this: https://astropy-healpix.readthedocs.io/en/latest/healpy_compat.html
I think that's fine, we should keep it as a compatibility and transition layer and for testing, and also mentioning it in the docs like that is OK. But if others think it would be better to hide it more from the docs, or if that is the preference of the healpy
maintainers, that seems OK to me as well.
@astrofrog @lpsinger @dstndstn all - please comment!