search

cds-astro / mocpy

Python library to easily create and manipulate MOCs (Multi-Order Coverage maps)
https://cds-astro.github.io/mocpy/
BSD 3-Clause "New" or "Revised" License
59 stars 33 forks source link

MOC.from_valued_healpix_cells is broken in mocpy 0.12.0 #91

Closed lpsinger closed 1 year ago

lpsinger commented 1 year ago

MOC.from_valued_healpix_cells is broken in mocpy 0.12.0.

Mocpy 0.12.0 had some changes with respect to the behavior of the max_depth keyword argument that seem like they are related. However, this keyword argument is still documented as being optional.

In mocpy 0.11.0:

$ python -c 'from mocpy import MOC; import numpy as np; print(MOC.from_valued_healpix_cells(np.asarray([16]), np.asarray([0])))'
[]

In mocpy 0.12.0:

$ python -c 'from mocpy import MOC; import numpy as np; print(MOC.from_valued_healpix_cells(np.asarray([16]), np.asarray([0])))'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/Users/lpsinger/src/ligo.skymap/.vscode/env/lib/python3.9/site-packages/mocpy/moc/moc.py", line 751, in from_valued_healpix_cells
    index = mocpy.from_valued_hpx_cells(
ValueError: Too deep cell depth. Expected: <= 0; Actual: 1

See lpsinger/ligo.skymap#19.

fxpineau commented 1 year ago

@lpsinger,

Yes it is a bug, we are going to fix it today. We first wanted to break the API requiring the max_depth parameter (it is a parameter provided in the multires FITS files, and providing it spares an extra loop on all cells). We then decided to do otherwise but we are not computing the max_depth at the right place... We will update both the code and the doc. Thank you for the notification.

fxpineau commented 1 year ago

Fixed by commit 5d09915, published in MOCPy v0.12.1. @lpsinger, @parkma99 be aware that the v0.12 contains breaking changes, see CHANGELOG for more details. Please let us know if you have questions/remarks.