Skip to content

fix: do not count punctuation-only tokens as words - #1871

Open
MohammadHijjawi97 wants to merge 3 commits into
Data-Centric-AI-Community:developfrom
MohammadHijjawi97:fix/word-counts-punctuation-tokens
Open

MohammadHijjawi97 wants to merge 3 commits into
Data-Centric-AI-Community:developfrom
MohammadHijjawi97:fix/word-counts-punctuation-tokens

Conversation

@MohammadHijjawi97

Copy link
Copy Markdown
Contributor

To be able to merge a pull request, there are a few checks:
[x] the black style should be enforced (make lint)
[ ] the docs should be regenerated (make docs)
[ ] the pull request should pass the tests (make test)
[ ] the examples should be generated (make examples)

Problem

word_summary_vc splits values on whitespace and strips punctuation from each token. Tokens made only of punctuation (-, --, ..., &) become empty strings and are counted as a word. The empty entry then shows up in the Words table of Categorical and Text columns, often at the top:

word_summary_vc(pd.Series(["hello - world", "foo -- bar!", "a ... b"]).value_counts())["word_counts"]
# '' -> 3, hello -> 1, world -> 1, ...

Solution

Drop empty tokens after stripping, before counting.

Tests

Added test_word_summary_vc_ignores_punctuation_only_tokens in tests/unit/test_pandas/test_describe_categorical_pandas.py; it fails before the change and passes after. Ran tests/unit/test_pandas and tests/unit/test_describe.py.

word_summary_vc strips punctuation from every whitespace-separated token,
so tokens such as "-" or "..." became empty strings and were counted as a
word. For text like "Star Wars - Episode IV" the empty word could become
the most common entry in the Words table. Drop the empty tokens before
counting.

@fabclmnt fabclmnt left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution!
Yes indeed, punctuation should not be included and the fix works for “-” and “...”. The root cause is that string. Nevertheless, punctuation is ASCII-only, so Unicode punctuation (em/en dashes, “…”, « », “ ”) still ends up as standalone “words” — “Star Wars — Episode IV” reproduces the original bug.

Could you strip with a Unicode-aware pattern instead, e.g. .str.replace(r"^[\W_]+|[\W_]+$", "", regex=True) and then drop empty tokens? That keeps inner punctuation (“don’t”, “e-mail”) as today.

Please also add test cases for “—”, “–”, “…” and a column where every value is punctuation-only (expect no word_counts), and update the TODO: remove punctuation

fabclmnt and others added 2 commits October 29, 2026 20:00
string.punctuation is ASCII-only, so em dashes, en dashes, ellipses and
curly or angle quotes were still counted as words. Strip leading and
trailing non-word characters with a Unicode-aware pattern instead, and
keep inner punctuation such as "don't" and "e-mail".
@MohammadHijjawi97

Copy link
Copy Markdown
Contributor Author

Thanks for the review. I switched the stripping to .str.replace(r"^[\W_]+|[\W_]+$", "", regex=True) and kept dropping empty tokens, so inner punctuation like "don't" and "e-mail" is unchanged, and I replaced the TODO with a comment.

I added tests for —, –, …, « », “ ” and for punctuation-only columns (no word_counts). They fail on the old code and pass now, and tests/unit passes apart from a parquet test that needs pyarrow, which passed once I installed it.

One thing to flag: \W also matches combining marks, so words ending in one are truncated (Hindi नमस्ते becomes नमस्त, and Thai is affected too), and emoji-only tokens are no longer counted. Let me know if you'd like that handled differently.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants