Back to report index

iolabs-common (shared config layer) 1bb1991: AI3D-382 Coerce number_of_returns to uint8 on load and reject bool arrays

Miroslav Simko <ms@iolabs.ch> 2026-09-01T11:09:17+02:00

Commit #86 ยท 6 snippets

 src/iolabs/common/segment_points_io.py | 36 +++++++++++++++----
 tests/test_segment_points_io.py        | 65 ++++++++++++++++++++++++++++++++++
 2 files changed, 94 insertions(+), 7 deletions(-)
Importance #1: src/iolabs/common/segment_points_io.py @@ -10,9 +10,12 @@
10uint8. It is **always written** by :func:`save_points_npz` -- a record that10uint8. It is **always written** by :func:`save_points_npz` -- a record that
11does not carry it is rejected rather than zero-filled, so a producer that11does not carry it is rejected rather than zero-filled, so a producer that
12still has the data cannot silently downgrade it. On the read side the key is12still has the data cannot silently downgrade it. On the read side the key is
13**optional**: datasets written before AI3D-382 have no such member, so13**optional**: datasets written before AI3D-382 have no such member, so
14:func:`load_points_npz` fills a zeros uint8 array of matching point count.14:func:`load_points_npz` fills a zeros uint8 array of matching point count; a
15member that *is* present is validated and cast to uint8 by the same rules, so
16both paths hand callers one dtype. Boolean arrays are rejected on either side:
17a mask is not a return count.
15Zero is not a legal LAS return count (valid values are 1-7 for point formats18Zero is not a legal LAS return count (valid values are 1-7 for point formats
160-5), which makes it an unambiguous "unknown" sentinel. Never default to 1 --190-5), which makes it an unambiguous "unknown" sentinel. Never default to 1 --
17that fabricates a plausible-looking measurement.20that fabricates a plausible-looking measurement.
1821
Importance #2: src/iolabs/common/segment_points_io.py @@ -75,15 +78,21 @@
75 Returns:78 Returns:
76 The values as uint8, unchanged when they already are.79 The values as uint8, unchanged when they already are.
7780
78 Raises:81 Raises:
79 ValueError: The array is not of integer dtype, or holds a value82 ValueError: The array is boolean or of non-integer dtype, or holds a
80 outside the uint8 range (which would wrap silently on cast).83 value outside the uint8 range (which would wrap silently on cast).
81 """84 """
82 array = np.asarray(values)85 array = np.asarray(values)
83 if array.dtype == NUMBER_OF_RETURNS_DTYPE:86 if array.dtype == NUMBER_OF_RETURNS_DTYPE:
84 return array87 return array
85 if array.dtype.kind not in "uib":88 if array.dtype.kind == "b":
89 raise ValueError(
90 f"{source}: '{NUMBER_OF_RETURNS_KEY}' must be an integer array, got a bool "
91 "array; a boolean is a mask, not a return count (casting it would fabricate "
92 "counts of 0 and 1)"
93 )
94 if array.dtype.kind not in "ui":
86 raise ValueError(95 raise ValueError(
87 f"{source}: '{NUMBER_OF_RETURNS_KEY}' must be an integer array, "96 f"{source}: '{NUMBER_OF_RETURNS_KEY}' must be an integer array, "
88 f"got dtype {array.dtype}"97 f"got dtype {array.dtype}"
89 )98 )
Importance #3: src/iolabs/common/segment_points_io.py @@ -105,10 +114,19 @@
105114
106 :data:`OPTIONAL_POINT_RECORD_KEYS` -- today just ``number_of_returns`` --115 :data:`OPTIONAL_POINT_RECORD_KEYS` -- today just ``number_of_returns`` --
107 may be absent: records written before AI3D-382 predate the key, and are116 may be absent: records written before AI3D-382 predate the key, and are
108 filled with a zeros uint8 array (0 = unknown, see the module docstring)117 filled with a zeros uint8 array (0 = unknown, see the module docstring)
109 rather than rejected. The returned mapping always holds every key in118 rather than rejected. When the key *is* present it is validated and cast
110 :data:`POINT_RECORD_KEYS`.119 to uint8 under the same rules :func:`save_points_npz` applies, so a
120 hand-rolled producer's wider (or boolean) dtype cannot leak downstream and
121 break a mixed-vintage concatenation. The returned mapping always holds
122 every key in :data:`POINT_RECORD_KEYS`, with ``number_of_returns`` always
123 uint8.
124
125 Raises:
126 ValueError: A required key is missing, an array has the wrong shape, or
127 ``number_of_returns`` is stored with a non-integer/bool dtype or
128 values outside the uint8 range.
111 """129 """
112 npz_path = Path(path)130 npz_path = Path(path)
113 with np.load(npz_path) as data:131 with np.load(npz_path) as data:
114 missing = [key for key in REQUIRED_POINT_RECORD_KEYS if key not in data.files]132 missing = [key for key in REQUIRED_POINT_RECORD_KEYS if key not in data.files]
Importance #4: src/iolabs/common/segment_points_io.py @@ -133,9 +151,13 @@
133 if arr.shape != (n_points,):151 if arr.shape != (n_points,):
134 raise ValueError(152 raise ValueError(
135 f"{npz_path}: '{key}' must have shape (N,), got {arr.shape}"153 f"{npz_path}: '{key}' must have shape (N,), got {arr.shape}"
136 )154 )
137 if NUMBER_OF_RETURNS_KEY not in record:155 if NUMBER_OF_RETURNS_KEY in record:
156 record[NUMBER_OF_RETURNS_KEY] = _coerce_number_of_returns(
157 record[NUMBER_OF_RETURNS_KEY], source=str(npz_path)
158 )
159 else:
138 logger.debug(160 logger.debug(
139 "%s: no '%s' member (pre-AI3D-382 record); filling %d zeros (unknown)",161 "%s: no '%s' member (pre-AI3D-382 record); filling %d zeros (unknown)",
140 npz_path.name,162 npz_path.name,
141 NUMBER_OF_RETURNS_KEY,163 NUMBER_OF_RETURNS_KEY,
Importance #5: tests/test_segment_points_io.py @@ -192,8 +192,73 @@
192 ):192 ):
193 load_points_npz(path)193 load_points_npz(path)
194194
195195
196def test_save_points_npz_rejects_bool_number_of_returns(tmp_path: Path) -> None:
197 """A bool mask is not a return count; casting it would fabricate 0/1 counts."""
198 record = _make_record(3, seed=68)
199 record[NUMBER_OF_RETURNS_KEY] = np.array([True, False, True])
200 with pytest.raises(ValueError, match="number_of_returns.*bool"):
201 save_points_npz(tmp_path / "out.npz", record)
202
203
204def test_load_points_npz_rejects_bool_number_of_returns(tmp_path: Path) -> None:
205 record = _make_record(3, seed=69)
206 record[NUMBER_OF_RETURNS_KEY] = np.array([True, False, True])
207 path = tmp_path / "bool_returns_run3_points.npz"
208 np.savez_compressed(path, **record)
209 with pytest.raises(ValueError, match="number_of_returns.*bool"):
210 load_points_npz(path)
211
212
213def test_load_points_npz_casts_stored_number_of_returns_to_uint8(tmp_path: Path) -> None:
214 """A hand-rolled producer's wider dtype must not leak into merges."""
215 record = _make_record(5, seed=70)
216 stored = record[NUMBER_OF_RETURNS_KEY].astype(np.int32)
217 record[NUMBER_OF_RETURNS_KEY] = stored
218 path = tmp_path / "int32_returns_run3_points.npz"
219 np.savez_compressed(path, **record)
220
221 loaded = load_points_npz(path)
222
223 assert loaded[NUMBER_OF_RETURNS_KEY].dtype == np.uint8
224 np.testing.assert_array_equal(loaded[NUMBER_OF_RETURNS_KEY], stored)
225
226
227def test_load_points_npz_rejects_out_of_range_stored_number_of_returns(
228 tmp_path: Path,
229) -> None:
230 record = _make_record(3, seed=71)
231 record[NUMBER_OF_RETURNS_KEY] = np.array([1, 300, 3], dtype=np.int32)
232 path = tmp_path / "out_of_range_returns_run3_points.npz"
233 np.savez_compressed(path, **record)
234 with pytest.raises(ValueError, match="number_of_returns.*fit in uint8"):
235 load_points_npz(path)
236
237
238def test_load_points_npz_rejects_float_stored_number_of_returns(tmp_path: Path) -> None:
239 record = _make_record(3, seed=72)
240 record[NUMBER_OF_RETURNS_KEY] = np.array([1.0, 2.0, 3.0], dtype=np.float32)
241 path = tmp_path / "float_returns_run3_points.npz"
242 np.savez_compressed(path, **record)
243 with pytest.raises(ValueError, match="number_of_returns.*integer array"):
244 load_points_npz(path)
245
246
247def test_concat_points_npz_merges_mixed_stored_return_dtypes(tmp_path: Path) -> None:
248 """int32-stored and uint8-stored records concatenate after the load coercion."""
249 wide = _make_record(3, seed=73)
250 wide[NUMBER_OF_RETURNS_KEY] = wide[NUMBER_OF_RETURNS_KEY].astype(np.int32)
251 wide_path = tmp_path / "alpha_run3_points.npz"
252 np.savez_compressed(wide_path, **wide)
253 narrow_path = save_points_npz(tmp_path / "beta_run3_points.npz", _make_record(2, seed=74))
254
255 merged, _ = concat_points_npz([wide_path, narrow_path])
256
257 assert merged[NUMBER_OF_RETURNS_KEY].dtype == np.uint8
258 assert merged[NUMBER_OF_RETURNS_KEY].shape == (5,)
259
260
196def test_load_segment_points_merges_legacy_and_new_records(tmp_path: Path) -> None:261def test_load_segment_points_merges_legacy_and_new_records(tmp_path: Path) -> None:
197 legacy = _make_legacy_record(3, seed=66)262 legacy = _make_legacy_record(3, seed=66)
198 modern = _make_record(4, seed=67)263 modern = _make_record(4, seed=67)
199 legacy_path = tmp_path / "alpha_run3_points.npz"264 legacy_path = tmp_path / "alpha_run3_points.npz"
Importance #6: tests/test_segment_points_io.py @@ -192,8 +192,73 @@
192 ):192 ):
193 load_points_npz(path)193 load_points_npz(path)
194194
195195
196def test_save_points_npz_rejects_bool_number_of_returns(tmp_path: Path) -> None:
197 """A bool mask is not a return count; casting it would fabricate 0/1 counts."""
198 record = _make_record(3, seed=68)
199 record[NUMBER_OF_RETURNS_KEY] = np.array([True, False, True])
200 with pytest.raises(ValueError, match="number_of_returns.*bool"):
201 save_points_npz(tmp_path / "out.npz", record)
202
203
204def test_load_points_npz_rejects_bool_number_of_returns(tmp_path: Path) -> None:
205 record = _make_record(3, seed=69)
206 record[NUMBER_OF_RETURNS_KEY] = np.array([True, False, True])
207 path = tmp_path / "bool_returns_run3_points.npz"
208 np.savez_compressed(path, **record)
209 with pytest.raises(ValueError, match="number_of_returns.*bool"):
210 load_points_npz(path)
211
212
213def test_load_points_npz_casts_stored_number_of_returns_to_uint8(tmp_path: Path) -> None:
214 """A hand-rolled producer's wider dtype must not leak into merges."""
215 record = _make_record(5, seed=70)
216 stored = record[NUMBER_OF_RETURNS_KEY].astype(np.int32)
217 record[NUMBER_OF_RETURNS_KEY] = stored
218 path = tmp_path / "int32_returns_run3_points.npz"
219 np.savez_compressed(path, **record)
220
221 loaded = load_points_npz(path)
222
223 assert loaded[NUMBER_OF_RETURNS_KEY].dtype == np.uint8
224 np.testing.assert_array_equal(loaded[NUMBER_OF_RETURNS_KEY], stored)
225
226
227def test_load_points_npz_rejects_out_of_range_stored_number_of_returns(
228 tmp_path: Path,
229) -> None:
230 record = _make_record(3, seed=71)
231 record[NUMBER_OF_RETURNS_KEY] = np.array([1, 300, 3], dtype=np.int32)
232 path = tmp_path / "out_of_range_returns_run3_points.npz"
233 np.savez_compressed(path, **record)
234 with pytest.raises(ValueError, match="number_of_returns.*fit in uint8"):
235 load_points_npz(path)
236
237
238def test_load_points_npz_rejects_float_stored_number_of_returns(tmp_path: Path) -> None:
239 record = _make_record(3, seed=72)
240 record[NUMBER_OF_RETURNS_KEY] = np.array([1.0, 2.0, 3.0], dtype=np.float32)
241 path = tmp_path / "float_returns_run3_points.npz"
242 np.savez_compressed(path, **record)
243 with pytest.raises(ValueError, match="number_of_returns.*integer array"):
244 load_points_npz(path)
245
246
247def test_concat_points_npz_merges_mixed_stored_return_dtypes(tmp_path: Path) -> None:
248 """int32-stored and uint8-stored records concatenate after the load coercion."""
249 wide = _make_record(3, seed=73)
250 wide[NUMBER_OF_RETURNS_KEY] = wide[NUMBER_OF_RETURNS_KEY].astype(np.int32)
251 wide_path = tmp_path / "alpha_run3_points.npz"
252 np.savez_compressed(wide_path, **wide)
253 narrow_path = save_points_npz(tmp_path / "beta_run3_points.npz", _make_record(2, seed=74))
254
255 merged, _ = concat_points_npz([wide_path, narrow_path])
256
257 assert merged[NUMBER_OF_RETURNS_KEY].dtype == np.uint8
258 assert merged[NUMBER_OF_RETURNS_KEY].shape == (5,)
259
260
196def test_load_segment_points_merges_legacy_and_new_records(tmp_path: Path) -> None:261def test_load_segment_points_merges_legacy_and_new_records(tmp_path: Path) -> None:
197 legacy = _make_legacy_record(3, seed=66)262 legacy = _make_legacy_record(3, seed=66)
198 modern = _make_record(4, seed=67)263 modern = _make_record(4, seed=67)
199 legacy_path = tmp_path / "alpha_run3_points.npz"264 legacy_path = tmp_path / "alpha_run3_points.npz"