Repository navigation
Conversation
LayoutElements.concatenate remapped class ids only for a group with a non-empty element_class_id_map. A group without one kept its raw ids, which then indexed the rebuilt map and picked up another group's class name. Give such a group's elements the class name None, the type iter_elements already reports for them and the name from_list uses for an element without a type. Fixes Unstructured-IO#523
chrikrah
left a comment
chrikrah
left a comment
There was a problem hiding this comment.
Head bcc4cae against base 158db67. The unmapped group keeps None instead of taking another group's name, and the suite gains exactly one test.
$ uv run --locked --no-sync pytest test_unstructured_inference/test_elements.py -q
51 passed, 1 skipped in 0.92s
$ git show 158db67ed9f6431e250047f4de07c2cbda4de611:unstructured_inference/inference/layoutelement.py > unstructured_inference/inference/layoutelement.py
$ uv run --locked --no-sync pytest test_unstructured_inference/test_elements.py -q
1 failed, 50 passed, 1 skipped in 0.95s
$ CI=true uv run --locked --no-sync pytest -n 4 -m "not slow" test_unstructured_inference -q
346 passed, 1 skipped, 1 warning in 24.19s # base 158db67 is 345 passed, 1 skipped
$ uv run --locked --no-sync make check
All checks passed! / 43 files already formatted / version sync would make no changes
# not run: the model-backed suites, which need the HF weights
History: layoutelement.py:116 dates to 9504649 and the remap block to 6895ddb, whose message is "fix bug when concatenating a list of LayoutElements the class id maps is not updated correctly". The map-less group was never handled, and no commit defends leaving it out.
non-blocking: element_class_id_map: dict[int, str] at layoutelement.py:26 now carries None as a value, so the annotation should read dict[int, str | None].
non-blocking: the test covers the unmapped group second. A group with no elements adds no slot, and two unmapped groups share one slot; both are worth a case.
@badGarnet you wrote concatenate and this takes the fresh-slot half of #523. I would keep None as the name in element_class_id_map: iter_elements already reads that map, so no lookup failure has to be added. Is None the name you want there?
Fixes #523.
LayoutElements.concatenateremapped class ids only for a group with a non-emptyelement_class_id_map. An unmapped group kept its raw ids, so they picked up another group's name from the rebuilt map.The issue left two options open: fresh slots or raise. This goes with a fresh slot named
None.iter_elementsalready returnstype=Nonefor an unmapped group, andfrom_listusesNoneas the name for an untyped element, so a concatenated unmapped group now reads the same as it did before the merge. It also shares theNoneslot with afrom_listgroup. A group with no elements adds no slot.Changes: the
elifbranch inconcatenate, one test intest_elements.py, and a 1.6.15 changelog entry with the version bump.@badGarnet, you wrote
concatenate; @CyMule, you were last in this file with #522. If you'd rather an unmapped group raise like__eq__does, I can switch the branch to aValueError. Which do you want?