Skip to content

Commit 854aa7d

Browse files
authored
Check arrow for nulls (#629)
1 parent 44488ee commit 854aa7d

3 files changed

Lines changed: 83 additions & 18 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
2525

2626
### Fixed
2727

28-
--
28+
- `.many(arrow_array)` now checks for nulls, and raises an arrow if any. Previously we used all the values, even masked, which may cause unexpected results https://github.com/light-curve/light-curve-python/pull/629
2929

3030
### Security
3131

light-curve/src/features.rs

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use crate::ln_prior::LnPrior1D;
88
use crate::np_array::Arr;
99
use crate::transform::{StockTransformer, parse_transform};
1010

11+
use arrow_array::Array;
1112
use arrow_array::cast::AsArray;
1213
use const_format::formatcp;
1314
use conv::ConvUtil;
@@ -593,13 +594,31 @@ impl PyFeatureEvaluator {
593594
fill_value: Option<T>,
594595
n_jobs: i64,
595596
) -> Res<ndarray::Array2<T>> {
596-
let chunks = chunked.chunks();
597-
598-
let tss = chunks
597+
let tss = chunked
598+
.chunks()
599599
.iter()
600-
.flat_map(|chunk| {
601-
let list = chunk.as_list::<O>();
600+
.map(|chunk| {
601+
let list: &arrow_array::GenericListArray<O> = chunk.as_list::<O>();
602+
// O(1) null checks — nulls are not supported yet
603+
if list.null_count() > 0 {
604+
return Err(Exception::NotImplementedError(
605+
"Null entries in the list array are not supported".to_string(),
606+
));
607+
}
602608
let struct_arr = list.values().as_struct();
609+
if struct_arr.null_count() > 0 {
610+
return Err(Exception::NotImplementedError(
611+
"Null entries in the struct array are not supported".to_string(),
612+
));
613+
}
614+
for i in 0..struct_arr.num_columns() {
615+
if struct_arr.column(i).null_count() > 0 {
616+
return Err(Exception::NotImplementedError(
617+
"Null values in data columns are not supported".to_string(),
618+
));
619+
}
620+
}
621+
603622
let t_vals: &[T] = struct_arr
604623
.column(0)
605624
.as_primitive::<T::ArrowType>()
@@ -618,19 +637,24 @@ impl PyFeatureEvaluator {
618637
.as_ref()
619638
});
620639
let offsets = list.value_offsets();
621-
offsets.iter().tuple_windows().map(move |(&start, &end)| {
622-
let (start, end) = (start.as_usize(), end.as_usize());
623-
Self::ts_from_views(
624-
feature_evaluator,
625-
ndarray::ArrayView1::from(&t_vals[start..end]),
626-
ndarray::ArrayView1::from(&m_vals[start..end]),
627-
sigma_vals.map(|s| ndarray::ArrayView1::from(&s[start..end])),
628-
sorted,
629-
check,
630-
is_t_required,
631-
)
632-
})
640+
offsets
641+
.iter()
642+
.tuple_windows()
643+
.map(move |(&start, &end)| {
644+
let (start, end) = (start.as_usize(), end.as_usize());
645+
Self::ts_from_views(
646+
feature_evaluator,
647+
ndarray::ArrayView1::from(&t_vals[start..end]),
648+
ndarray::ArrayView1::from(&m_vals[start..end]),
649+
sigma_vals.map(|s| ndarray::ArrayView1::from(&s[start..end])),
650+
sorted,
651+
check,
652+
is_t_required,
653+
)
654+
})
655+
.collect::<Res<Vec<_>>>()
633656
})
657+
.flatten_ok()
634658
.collect::<Res<Vec<_>>>()?;
635659

636660
if tss.is_empty() {

light-curve/tests/light_curve_ext/test_feature.py

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -668,3 +668,44 @@ def test_many_arro3():
668668
arro3_arr = arro3.core.Array.from_arrow(arrow_arr)
669669
result = feature.many(arro3_arr, sorted=True, n_jobs=1)
670670
assert_array_equal(expected, result)
671+
672+
673+
@pytest.mark.parametrize(
674+
"arrow_arr, match_msg",
675+
[
676+
# Null list entry (whole light curve is null)
677+
(
678+
pa.array(
679+
[[{"t": 1.0, "m": 2.0, "sigma": 0.1}], None],
680+
type=pa.list_(pa.struct([("t", pa.float64()), ("m", pa.float64()), ("sigma", pa.float64())])),
681+
),
682+
"list array",
683+
),
684+
# Null struct entry (observation within a light curve is null)
685+
(
686+
pa.ListArray.from_arrays(
687+
pa.array([0, 2], type=pa.int32()),
688+
pa.StructArray.from_arrays(
689+
[pa.array([1.0, 2.0]), pa.array([3.0, 4.0]), pa.array([0.1, 0.2])],
690+
names=["t", "m", "sigma"],
691+
mask=pa.array([False, True]),
692+
),
693+
),
694+
"struct array",
695+
),
696+
# Null value in a data column
697+
(
698+
pa.array(
699+
[[{"t": 1.0, "m": None, "sigma": 0.1}, {"t": 2.0, "m": 5.0, "sigma": 0.2}]],
700+
type=pa.list_(pa.struct([("t", pa.float64()), ("m", pa.float64()), ("sigma", pa.float64())])),
701+
),
702+
"data columns",
703+
),
704+
],
705+
ids=["null_list_entry", "null_struct_entry", "null_value"],
706+
)
707+
def test_many_arrow_nulls_rejected(arrow_arr, match_msg):
708+
"""Null values at any level of the Arrow array raise NotImplementedError."""
709+
feature = lc.Amplitude()
710+
with pytest.raises(NotImplementedError, match=match_msg):
711+
feature.many(arrow_arr, sorted=True, n_jobs=1)

0 commit comments

Comments
 (0)