Skip to content

Commit 434184a

Browse files
kurodo3[bot]claude
andcommitted
fix(context): repair v0.1.json merge conflict; move _optional None filtering to parse_objectspec
Fix two missing commas left by the merge conflict resolution in v0.1.json (SI handler entry lacked comma before numpy.ndarray; SI changelog entry lacked comma before pandas entry). Also address review feedback on the `not None` check in LogicalTypeRegistry: move the filtering of optional-missing entries to parse_objectspec (where None is produced) so registries receive clean lists. parse_objectspec now drops None results from "_optional": true dict items when processing lists. For handler pairs (inner 2-element lists), optional filtering makes the inner list empty []; PythonTypeHandlerRegistry guards with `entry and len(entry) == 2` to correctly skip both empty and incomplete pairs. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 9b1de7e commit 434184a

5 files changed

Lines changed: 59 additions & 23 deletions

File tree

src/orcapod/contexts/data/v0.1.json

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,7 @@
107107
[{"_type": "typing._SpecialForm"}, {"_class": "orcapod.hashing.semantic_hashing.builtin_handlers.SpecialFormHandler", "_config": {}}],
108108
[{"_type": "pyarrow.Table"}, {"_class": "orcapod.hashing.semantic_hashing.builtin_handlers.ArrowTableHandler", "_config": {}}],
109109
[{"_type": "pyarrow.RecordBatch"}, {"_class": "orcapod.hashing.semantic_hashing.builtin_handlers.ArrowTableHandler", "_config": {}}],
110-
[{"_type": "spikeinterface.core.BaseRecording", "_optional": true}, {"_class": "orcapod.extension_types.spikeinterface_types.SIRecordingHandler", "_config": {}, "_optional": true}]
110+
[{"_type": "spikeinterface.core.BaseRecording", "_optional": true}, {"_class": "orcapod.extension_types.spikeinterface_types.SIRecordingHandler", "_config": {}, "_optional": true}],
111111
[{"_type": "numpy.ndarray"}, {"_class": "orcapod.hashing.semantic_hashing.builtin_handlers.NumpyArrayHandler", "_config": {}}],
112112
[{"_type": "pandas.DataFrame"}, {"_class": "orcapod.hashing.semantic_hashing.builtin_handlers.PandasDataFrameHandler", "_config": {}}],
113113
[{"_type": "pandas.Series"}, {"_class": "orcapod.hashing.semantic_hashing.builtin_handlers.PandasSeriesHandler", "_config": {}}]
@@ -144,7 +144,7 @@
144144
"Migrated LogicalFile Arrow storage from plain path string to JSON {\"path\": \"...\"} for consistency with LogicalDirectory (ITL-451)",
145145
"Added orcapod.Directory content-identified type with recursive Merkle tree hashing, LogicalDirectory Arrow extension, ignore filter support, and DirectoryHandler (ITL-451)",
146146
"Added numpy.ndarray as a native value type via LogicalNumpyArray (large_binary/.npy) and NumpyArrayHandler; object-dtype arrays are rejected eagerly (ITL-460)",
147-
"Added spikeinterface.BaseRecording as a native value type via LogicalSIRecording (large_string/JSON) and SIRecordingHandler (SHA-256 of JSON bytes); auto-registered when spikeinterface is installed via _optional entries in v0.1.json; added _optional flag support to parse_objectspec for optional-extras types (ITL-459)"
147+
"Added spikeinterface.BaseRecording as a native value type via LogicalSIRecording (large_string/JSON) and SIRecordingHandler (SHA-256 of JSON bytes); auto-registered when spikeinterface is installed via _optional entries in v0.1.json; added _optional flag support to parse_objectspec for optional-extras types (ITL-459)",
148148
"Added pandas.DataFrame and pandas.Series as native value types via LogicalPandasDataFrame and LogicalPandasSeries (Arrow IPC / large_binary), with index preservation and PandasDataFrameHandler / PandasSeriesHandler for content hashing using StarfixArrowHasher (PLT-1869)"
149149
]
150150
}

src/orcapod/extension_types/registry.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -218,8 +218,7 @@ def __init__(
218218
self._category_factories: dict[str, LogicalTypeFactoryProtocol] = {}
219219
self._python_class_factories: dict[type, LogicalTypeFactoryProtocol] = {}
220220
for lt in (logical_types or []):
221-
if lt is not None:
222-
self.register_logical_type(lt)
221+
self.register_logical_type(lt)
223222
for entry in (factories or []):
224223
self.register_logical_type_factory(
225224
entry["factory"],

src/orcapod/hashing/semantic_hashing/type_handler_registry.py

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -45,10 +45,12 @@ def __init__(
4545
self._lock = threading.RLock()
4646
if handlers:
4747
for entry in handlers:
48-
# Skip entries where either element resolved to None — this happens
49-
# when a handler pair carries "_optional": true and the backing
50-
# module (e.g. an extras-group dependency) is not installed.
51-
if entry is not None and all(x is not None for x in entry):
48+
# Skip empty or incomplete pairs. When a handler pair in the context
49+
# JSON carries "_optional": true on its elements and the backing module
50+
# is absent, parse_objectspec filters those elements out of the inner
51+
# list, leaving an empty list [] here. Also skip pairs where either
52+
# element resolved to None.
53+
if entry and len(entry) == 2 and all(x is not None for x in entry):
5254
target_type, handler = entry
5355
self.register(target_type, handler)
5456

src/orcapod/utils/object_spec.py

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,17 @@ def parse_objectspec(
2929
}
3030

3131
elif isinstance(obj_spec, (list, tuple)):
32-
processed = [parse_objectspec(item, ref_lut, validate) for item in obj_spec]
32+
processed = []
33+
for item in obj_spec:
34+
result = parse_objectspec(item, ref_lut, validate)
35+
# Omit None results produced by dict items with "_optional": true that
36+
# failed to resolve due to a missing dependency. This allows entries
37+
# like LogicalSIRecording (backed by an optional extras group) to be
38+
# listed in a context JSON without breaking startup when the extras are
39+
# absent — they are simply absent from the resulting list.
40+
if result is None and isinstance(item, dict) and item.get("_optional", False):
41+
continue
42+
processed.append(result)
3343
return tuple(processed) if isinstance(obj_spec, tuple) else processed
3444

3545
else:

tests/test_utils/test_object_spec.py

Lines changed: 39 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -93,19 +93,17 @@ def test_optional_false_still_raises(self, monkeypatch):
9393
# ---------------------------------------------------------------------------
9494

9595
class TestParseObjectspecOptional:
96-
def test_list_with_optional_missing_filters_none(self, monkeypatch):
97-
"""A list of specs where one optional entry is missing returns None for it."""
96+
def test_list_with_optional_missing_drops_entry(self, monkeypatch):
97+
"""Optional dict items that fail to resolve are dropped from the list entirely."""
9898
monkeypatch.setitem(sys.modules, "nonexistent_pkg_xyz", None)
9999
specs = [
100100
{"_type": "builtins.int"},
101101
{"_type": "nonexistent_pkg_xyz.SomeClass", "_optional": True},
102102
{"_type": "builtins.str"},
103103
]
104104
result = parse_objectspec(specs)
105-
# None appears in position 1 because the module was absent
106-
assert result[0] is int
107-
assert result[1] is None
108-
assert result[2] is str
105+
# The missing optional entry is dropped; the list has 2 elements, not 3.
106+
assert result == [int, str]
109107

110108
def test_class_spec_with_optional_missing_returns_none(self, monkeypatch):
111109
monkeypatch.setitem(sys.modules, "nonexistent_pkg_xyz", None)
@@ -127,15 +125,29 @@ def test_class_spec_without_optional_raises(self, monkeypatch):
127125
# LogicalTypeRegistry — None filtering
128126
# ---------------------------------------------------------------------------
129127

130-
class TestLogicalTypeRegistryNoneFiltering:
131-
def test_none_in_logical_types_is_skipped(self):
132-
from orcapod.extension_types.registry import LogicalTypeRegistry
133-
from orcapod.extension_types.builtin_logical_types import LogicalUUID
128+
class TestParseObjectspecOptionalListFiltering:
129+
def test_optional_class_spec_filtered_from_list(self, monkeypatch):
130+
"""_optional _class items that fail to resolve are dropped from the list."""
131+
monkeypatch.setitem(sys.modules, "nonexistent_pkg_xyz", None)
132+
specs = [
133+
{"_type": "builtins.int"},
134+
{"_class": "nonexistent_pkg_xyz.SomeClass", "_config": {}, "_optional": True},
135+
{"_type": "builtins.str"},
136+
]
137+
result = parse_objectspec(specs)
138+
assert result == [int, str]
134139

135-
# None should be silently skipped; valid type should still register.
136-
registry = LogicalTypeRegistry(logical_types=[None, LogicalUUID()])
137-
lt = registry.get_by_logical_name("orcapod.uuid")
138-
assert lt is not None
140+
def test_optional_class_spec_included_when_present(self):
141+
"""_optional _class items that resolve successfully are included."""
142+
specs = [
143+
{"_type": "builtins.int"},
144+
{"_class": "builtins.list", "_config": {}, "_optional": True},
145+
{"_type": "builtins.str"},
146+
]
147+
result = parse_objectspec(specs)
148+
assert result[0] is int
149+
assert isinstance(result[1], list)
150+
assert result[2] is str
139151

140152

141153
# ---------------------------------------------------------------------------
@@ -157,6 +169,19 @@ def test_none_pair_in_handlers_is_skipped(self):
157169
handler = registry.get_handler(b"hello")
158170
assert handler is not None
159171

172+
def test_empty_pair_in_handlers_is_skipped(self):
173+
"""Empty list entries (optional pair where all elements were filtered) are skipped."""
174+
from orcapod.hashing.semantic_hashing.type_handler_registry import PythonTypeHandlerRegistry
175+
from orcapod.hashing.semantic_hashing.builtin_handlers import BytesHandler
176+
177+
# parse_objectspec filters _optional elements from the inner list, leaving [].
178+
handlers = [
179+
(bytes, BytesHandler()),
180+
[], # inner list became empty after optional filtering
181+
]
182+
registry = PythonTypeHandlerRegistry(handlers=handlers)
183+
assert registry.get_handler(b"x") is not None
184+
160185
def test_none_entry_itself_skipped(self):
161186
from orcapod.hashing.semantic_hashing.type_handler_registry import PythonTypeHandlerRegistry
162187
from orcapod.hashing.semantic_hashing.builtin_handlers import BytesHandler

0 commit comments

Comments
 (0)