Skip to content

Commit

Permalink
fix: add checks before casting indices to unsigned integers
Browse files Browse the repository at this point in the history
  • Loading branch information
ManonMarchand committed Jul 5, 2024
1 parent 0a3a289 commit b721976
Show file tree
Hide file tree
Showing 3 changed files with 104 additions and 31 deletions.
7 changes: 7 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,18 @@ between the small MOC described by the polygon or the bigger one (its complement

### Changed

* `MOC.from_healpix_cells` also accepts an int as depth if all the cells are at the same level
* `MOC.from_vizier_table()` does not call the MOCServer anymore. It now raises an error if the
catalog of table name is invalid (see #143). It also accepts `max_depth` as an argument. This
should replace `nside` in a future version.
* `MOC.from_ivorn()` now accepts `max_depth` as an argument. This should reb=place `nside` later.

### Fixed

* `ranges` in `from_depth29_ranges` is now optional, to be consistent with the existing docstring
* `from_healpix_cells` and `from_velued_healpix_cells` now filter out invalid cells and raise a
warning when they do so

## [0.15.0]

### Added
Expand Down
50 changes: 43 additions & 7 deletions python/mocpy/moc/moc.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import contextlib
import functools
import warnings
from collections.abc import Iterable
from io import BytesIO
from math import log2
from pathlib import Path
Expand Down Expand Up @@ -80,6 +81,28 @@ def _validate_lonlat_wrap(self, lon, lat, **kwargs):
return _validate_lonlat_wrap


def _mask_unsigned_before_casting(indices):
"""Return a mask for an array of integers if there are negative values.
This is useful before casting indices into unsigned integers.
Parameters
----------
indices : `numpy.ndarray` or Iterable
"""
if np.issubdtype(np.asarray(indices).dtype, np.unsignedinteger) or all(
np.asarray(indices) > 0,
):
return None
warnings.warn(
"The list of indices contain negative values. They are filtered "
"out to generate the MOC",
UserWarning,
stacklevel=2,
)
return np.array(indices) > 0


class MOC(AbstractMOC):
"""
Multi-order spatial coverage class.
Expand Down Expand Up @@ -132,8 +155,7 @@ def __init__(self, store_index):
@property
def max_order(self):
"""Depth/order of the S-MOC."""
depth = mocpy.get_smoc_depth(self.store_index)
return np.uint8(depth)
return mocpy.get_smoc_depth(self.store_index)

@classmethod
def n_cells(cls, depth):
Expand Down Expand Up @@ -1049,6 +1071,12 @@ def from_valued_healpix_cells(
"Invalid uniq numbers. Too big uniq or negative uniq numbers might be the cause.",
)

mask = _mask_unsigned_before_casting(uniq)
# if any of the values in uniq are negative
if mask is not None:
uniq = np.array(uniq)[mask]
values = np.array(values)[mask]

index = mocpy.from_valued_hpx_cells(
np.uint8(max_depth),
uniq.astype(np.uint64),
Expand Down Expand Up @@ -1755,7 +1783,7 @@ def from_healpix_cells(cls, ipix, depth, max_depth):
----------
ipix : `numpy.ndarray`
HEALPix cell indices in the NESTED notation. dtype must be np.uint64
depth : `numpy.ndarray`
depth : `numpy.ndarray` or int
Depth of the HEALPix cells. Must be of the same size of `ipix`.
dtype must be np.uint8. Corresponds to the `level` of an HEALPix cell in astropy.healpix.
max_depth : int, The resolution of the MOC (degrades on the fly input cells if necessary)
Expand All @@ -1767,18 +1795,26 @@ def from_healpix_cells(cls, ipix, depth, max_depth):
Returns
-------
moc : `~mocpy.moc.MOC`
`~mocpy.moc.MOC`
The MOC
"""
if not isinstance(depth, Iterable):
depth = np.full(len(ipix), depth, dtype=np.uint8)

mask = _mask_unsigned_before_casting(ipix)
if mask is not None:
ipix = np.array(ipix)[mask]
depth = np.array(depth)[mask]

index = mocpy.from_healpix_cells(
np.uint8(max_depth),
depth.astype(np.uint8),
ipix.astype(np.uint64),
np.array(depth, dtype=np.uint8),
np.array(ipix, dtype=np.uint64),
)
return cls(index)

@classmethod
def from_depth29_ranges(cls, max_depth, ranges):
def from_depth29_ranges(cls, max_depth, ranges=None):
"""
Create a MOC from a set of ranges of HEALPix Nested indices at order 29.
Expand Down
78 changes: 54 additions & 24 deletions python/mocpy/tests/test_moc.py
Original file line number Diff line number Diff line change
Expand Up @@ -230,24 +230,34 @@ def gen_f(size):
return gen_f


@pytest.mark.parametrize("size", [1000, 10000, 50000])
def test_moc_from_skycoords(skycoords_gen_f, size):
skycoords = skycoords_gen_f(size)
MOC.from_skycoords(skycoords, max_norder=7)
def test_moc_from_skycoords(skycoords_gen_f):
skycoords = skycoords_gen_f(10)
moc = MOC.from_skycoords(skycoords, max_norder=7)
assert all(moc.contains_skycoords(skycoords))


@pytest.mark.parametrize("size", [1000, 10000, 50000])
def test_moc_from_lonlat(lonlat_gen_f, size):
lon, lat = lonlat_gen_f(size)
MOC.from_lonlat(lon=lon, lat=lat, max_norder=6)
def test_moc_from_lonlat(lonlat_gen_f):
lon, lat = lonlat_gen_f(10)
moc_lon_lat = MOC.from_lonlat(lon=lon, lat=lat, max_norder=6)
moc_skycoo = MOC.from_skycoords(SkyCoord(lon, lat), max_norder=6)
assert moc_lon_lat == moc_skycoo


def test_from_healpix_cells():
ipix = np.array([40, 87, 65])
depth = np.array([3, 3, 3])
np.array([True, True, True])

MOC.from_healpix_cells(max_depth=29, ipix=ipix, depth=depth)
moc = MOC.from_healpix_cells(max_depth=3, ipix=ipix, depth=depth)
# the MOC is correct
assert moc == MOC.from_str("3/40 87 65")
# we can also give depth as an integer if the cells have only one order
assert moc == MOC.from_healpix_cells(ipix=ipix, depth=3, max_depth=3)
# negative indices are ignored
with pytest.warns(
UserWarning,
match="The list of indices contain negative values*",
):
moc = MOC.from_healpix_cells(ipix=[40, -1, 65], depth=depth, max_depth=3)
assert moc == MOC.from_str("3/40 65")


def test_from_polygons():
Expand Down Expand Up @@ -822,30 +832,50 @@ def test_spatial_res_to_order():
assert (order == output).all()


def test_from_valued_healpix_cells_empty():
uniq = np.array([])
values = np.array([])

MOC.from_valued_healpix_cells(uniq, values, 12)

def test_from_valued_healpix_cells():
uniq = np.array([8, 12])
values = np.array([0.4, 0.6])
moc_1 = MOC.from_valued_healpix_cells(
uniq,
values,
cumul_from=0,
cumul_to=1,
max_depth=12,
)
assert moc_1 == MOC.from_string("0/4 8\n12/")
moc_0p7 = MOC.from_valued_healpix_cells(
uniq,
values,
cumul_from=0,
cumul_to=0.7,
max_depth=12,
)
assert moc_0p7 == MOC.from_string("0/8\n12/")

def test_from_valued_healpix_cells_different_sizes():
# different sizes
uniq = np.array([500])
values = np.array([])

with pytest.raises(
ValueError,
match="`uniq` and values do not have the same size.",
):
MOC.from_valued_healpix_cells(uniq, values, 12)


def test_from_valued_healpix_cells_cumul_from_sup_cumul_to():
uniq = np.array([500])
# failed comparison between cumul_from and cumul_to
values = np.array([1.0])

with pytest.raises(ValueError, match="`cumul_from` has to be < to `cumul_to`."):
MOC.from_valued_healpix_cells(uniq, values, 12, cumul_from=0.8, cumul_to=-5.0)
# negative uniq
uniq = [-2, 270]
values = [0.2, 0.2]
with pytest.warns(
UserWarning,
match="The list of indices contain negative values*",
):
assert MOC.from_valued_healpix_cells(
uniq,
values,
max_depth=3,
) == MOC.from_string("3/14")


@pytest.mark.parametrize(
Expand Down

0 comments on commit b721976

Please sign in to comment.