Repository navigation
fix(refine): load the stopword corpus once and tolerate missing cells - #564
Open
cyberspace-cs wants to merge 1 commit into
Open
cyberspace-cs wants to merge 1 commit into
cyberspace-cs wants to merge 1 commit into
Conversation
RemoveStopwordsRefiner rebuilt set(stopwords.words("english")) inside the
per-row helper, so every row re-read and re-parsed the corpus before doing
any work. On 20k rows this costs 17.0s of pure overhead (0.85 ms/row); with
the set built once in __init__ the same run takes 0.08s (0.004 ms/row).
A null cell also reached text.split() and raised AttributeError, aborting
the whole step. Missing cells now pass through unchanged, matching how the
other general_text refiners keep rows aligned.
Contributor
Author
|
Friendly bump for review, @SunnyHaze — CI has been green on all six matrix jobs since 2026-10-20 00:48 and the branch is still Recap of the two things this touches, in case it helps prioritise:
If the two concerns would be easier to review separately, I am happy to split the NaN handling out into its own PR. |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this fixes
RemoveStopwordsRefinerrebuilt the English stopword set for every row:stopwords.words()opens and parses the corpus file on each call, so a table of n rows performs n file reads and n set constructions before a single token is filtered. The corpus is static for the lifetime of the operator, so it only needs to be read once.Measured on 20,000 rows (single sentence per row,
LazyFileStorage+ jsonl, Windows / Python 3.13):Roughly 210× faster on this workload, and the per-row cost no longer depends on the corpus at all.
Second fix: missing cells abort the step
A null cell (
None/NaN) reachestext.split()and raisesAttributeError: 'float' object has no attribute 'split', which aborts the whole refine step — one missing value kills the run. Missing cells are now passed through unchanged, which keeps the output row-aligned with the input, the same wayWordNumberFilter(#563) and the othergeneral_textrefiners behave.Tests
New file
test/cpu_only/test_remove_stopwords_refiner.py, three cases:test_stopwords_are_removed— normal text still filters as before.test_missing_cells_are_preserved—NoneandNaNsurvive and the row count is preserved.test_corpus_is_read_once_per_operator— patchesstopwords.wordsand asserts it is called exactly once per operator, so the per-row rebuild cannot silently come back.On
main, two of the three fail (the null cell raises, and the corpus counter records one load per row); with the patch:No API or output-schema change: the operator still returns
[input_key]and writes the refined text back into the same column.