Repository navigation
fix(modules): treat DocumentSplitter separator as a literal string - #2376
sarahyurick merged 7 commits into
Conversation
pandas Series.str.split interprets multi-character patterns as regular expressions by default, so separators like "..", "||" or "(sep)" split documents on the wrong boundaries, and invalid regexes like "++" crash the stage with re.error. DocumentSplitter documents a plain separator and DocumentJoiner joins with the literal separator, so the round-trip between the two stages was also broken for such separators. Pass regex=False so the separator always matches literally. Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
 |
svcnvidia-nemo-ci
added
the
waiting-on-maintainers
| reconstructed_doc2 = "\n\n".join(doc2_segments["text"].tolist()) | ||
| assert reconstructed_doc2 == "Foo\n\nBar" | ||
|
|
||
| def test_separator_is_treated_as_literal_string(self): |
There was a problem hiding this comment.
Hi @ManoharPaturi thanks for opening! I was wondering is there any way for you to create a single parametrized test instead of adding 3 separate tests?
There was a problem hiding this comment.
Thanks @sarahyurick! That makes total sense I've consolidated the cases into a single @pytest.mark.parametrize test (test_literal_separator_and_joiner_roundtrip) covering multi-character metacharacters (..), invalid regexes (++), and groups ((sep)) along with the DocumentJoiner round trip. Formatted with Ruff and pushed in 0c2ceb4.
svcnvidia-nemo-ci
added
waiting-on-customer
Signed-off-by: Manohar Paturi <186662190+ManoharPaturi@users.noreply.github.com>
svcnvidia-nemo-ci
added
waiting-on-maintainers
|
/ok to test c1ba87d |
|
/ok to test 844e6f4 |
|
/ok to test a28ffab |
Fixes #2374.
One line: pass
regex=Falseto thestr.splitcall so the separator is treated as a literal string, matching the docstring and theDocumentJoinerround-trip (which always joins literally)."..","||","(sep)"now split on the actual separator"++"no longer raisesre.error3 new tests (metacharacter split,
++crash, DocumentJoiner round-trip) — all fail on main, pass here. Rest of the splitter suite unchanged.