Repository navigation
test(tabulate): hide wcwidth from the reference import - #229
Conversation
The reference uses wcwidth for column widths when it is installed; our default is len(). The guarded import only hid the repo's own tabulate/ from the reference, so with wcwidth as a transitive dependency four unicode/CJK comparisons measured two different width rules and failed (#228). Hide wcwidth for the reference import and assert it stayed hidden.
![milo-oaklight[bot]](https://avatars.githubusercontent.com/in/3956677?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reproduced on master, fixed here, and the guard is the load-bearing part.
tabulate correctness file master this head
wcwidth absent 83 passed 84 passed (+1 guard test)
wcwidth installed 4 failed 84 passed
79 passed
whole tabulate package, 3.10 and 3.12, wcwidth present, benchmark plugin installed
90 passed 90 passed
The four that fail on master are exactly the width-sensitive ones: test_unicode_basic, test_cjk_characters, test_mixed_cjk_ascii, test_wide_emoji. So the diagnosis is right — nothing changed in either implementation, the environment changed under the test.
The guard test holds. Removing only the sys.modules['wcwidth'] = None line from this head gives:
FAILED test_reference_width_mode_is_len
FAILED test_unicode_basic
test_cjk_characters / test_mixed_cjk_ascii / test_wide_emoji
5 failed, 79 passed
The assertion on _ref_tabulate_mod.wcwidth is None is what makes the hiding non-optional; without it the four comparisons would silently go back to comparing two width rules the moment wcwidth appears transitively (which is how this landed on a green CI in the first place). Restoring the previous sys.modules state in finally is right too — poisoning a module name is a process-wide side effect.
One gap, non-blocking, for the record: WIDE_CHARS_MODE now has no coverage at all. This fix correctly aligns both sides on the len() path, but our wide path counts with unicodedata.east_asian_width while the reference counts with wcwidth, so those two disagree on combining marks and ZWJ emoji — and nothing tests our wide path against either. That is a separate issue from #228 (the mismatch being fixed here) and I would not bolt it onto this PR; worth an issue if wide mode is meant to be supported.
CI green on 3.10-3.13 plus lint and GitGuardian. Approving.
![elena-oaklight[bot]](https://avatars.githubusercontent.com/in/4082499?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reproduced and approved. The guard is the right lever: the divergence is the reference's width mode, not its width implementation.
Where the 4 failures come from
With wcwidth 0.9.3 and reference tabulate 0.10.0 installed:
base a37e656 4 failed, 85 passed
test_unicode_basic
test_cjk_characters
test_mixed_cjk_ascii
test_wide_emoji
head 9d34b8f 90 passed
Exactly the unicode/CJK set, which is what WIDE_CHARS_MODE differences predict - the reference switches to wcwidth.wcswidth when the package imports, ours defaults to len(s).
The guard test has teeth
Deleting sys.modules["wcwidth"] = None reddens five tests: the four comparisons again, plus test_reference_width_mode_is_len. So the new test is not a tautology - it detects the guard being removed.
Restore is clean
wcwidth cached before the import -> sys.modules['wcwidth'] is wcwidth after it (True)
wcwidth not cached before -> 'wcwidth' in sys.modules after it (False)
No leftover None entry, and the reference module ends up with wcwidth is None, so both sides compute widths with len. CI green on 3.10-3.13 plus lint.
Two coverage notes
- The guard is untested in CI. CI installs neither
wcwidthnor a transitive dependency that brings it, so_ref_tabulate_mod.wcwidthisNonethere regardless of the guard - the new test passes even if the guard line is deleted. That is how #228 reachedmainin the first place: the environment that would have caught it is not the environment CI builds. Addingwcwidthto thetestextra makes CI take the path that used to fail. - The guard hides a real difference. A user with
wcwidthinstalled gets wider output from the reference and from ourWIDE_CHARS_MODE=Truebuild; those two are no longer compared anywhere. If that matters, a conditional case that turnsWIDE_CHARS_MODEon for both sides whenwcwidthis importable would keep the comparison honest instead of removing it.
![clementine-oaklight[bot]](https://avatars.githubusercontent.com/in/3956685?s=60&v=4){kind=small}
There was a problem hiding this comment.
Reproduced both sides; the guard works, and there is one thing worth adding rather than removing.
The failure, tabulate/ with wcwidth 0.9.3 and tabulate 0.10.0 installed:
master a37e656 4 failed, 85 passed
TestEdgeCases::test_unicode_basic
TestEdgeCases::test_cjk_characters
TestEdgeCases::test_mixed_cjk_ascii
TestEdgeCases::test_wide_emoji
this head 90 passed (the four above plus the new guard test)
The guard test has teeth. With sys.modules["wcwidth"] = None replaced by pass:
5 failed, 79 passed
test_reference_width_mode_is_len + the same four comparisons
so the added test fails exactly when the hide stops working, which is what you want from a guard.
It restores cleanly. I executed the module against a live wcwidth:
reference tabulate.wcwidth attr None (the comparison really runs in len mode)
sys.modules["wcwidth"] restored True
fresh import wcwidth same object (nothing downstream loses it)
In an environment without wcwidth the module is 84 passed / 6 errors, and those six are tabulate/test_tabulate_benchmark.py asking for the benchmark fixture - the same missing-pytest-benchmark noise #224 fixed for jsonx with pytest.importorskip. Not this PR's problem, but if the intent is that a bare checkout stays quiet, the other benchmark modules are still waiting for the same one-liner.
The substantive note: hiding wcwidth removes the only coverage of the configuration a user actually has. A user installing the reference and comparing against zerodep has wcwidth present, and the reference then detects wide characters automatically while this module needs WIDE_CHARS_MODE. That is a real, documented difference (docs/modules/tabulate.md: "CJK support | Yes (via WIDE_CHARS_MODE) | Yes"), and after this PR nothing pins it. I measured the mapping on the four failing cases:
case default(len) equal WIDE_CHARS_MODE=True vs ref+wcwidth
unicode_basic no yes
cjk no yes
mixed_cjk_ascii no yes
wide_emoji no yes
ref + wcwidth zerodep WIDE_CHARS_MODE=True
---- ---- ---- ----
名前 年齢 名前 年齢
太郎 25 太郎 25
花子 30 花子 30
---- ---- ---- ----
The behaviour is implemented; only the assertion is missing. A test that imports the reference with wcwidth visible, flips WIDE_CHARS_MODE = True on our side, and compares those four cases would preserve the coverage this PR otherwise trades away - and it would catch a regression in the wide-char width function, which no other test touches.
Approving; the extra test is a suggestion, not a condition.
| from tabulate import tabulate as our_tabulate # noqa: E402 | ||
|
|
||
|
|
||
| def test_reference_width_mode_is_len() -> None: |
There was a problem hiding this comment.
This is the one thing the PR trades away rather than fixes: with wcwidth hidden, no test compares our output against the reference in the configuration a user has (reference auto-detects wide characters, we need WIDE_CHARS_MODE). That configuration is currently correct - I measured WIDE_CHARS_MODE = True against the reference-with-wcwidth on all four cases in this file and they match, while the default differs on all four.
ref + wcwidth zerodep WIDE_CHARS_MODE=True
---- ---- ---- ----
名前 年齢 名前 年齢
---- ---- ---- ----
So an extra test that imports the reference with wcwidth visible, sets WIDE_CHARS_MODE = True, and compares the four unicode/CJK cases would pin the mapping the docs already advertise (docs/modules/tabulate.md, "CJK support | Yes (via WIDE_CHARS_MODE)") and would be the only test covering the wide-char width path at all.
Review note on #229: hiding wcwidth made the default comparison environment-independent but dropped the only coverage of the wide-character width path. Adding it back exposed a real gap — WIDE_CHARS_MODE counted a combining accent as one column, so 'café' measured five where wcwidth reports four. Combining marks now contribute zero width, and a parametrised test compares WIDE_CHARS_MODE=True against the reference imported with wcwidth visible.
|
Clementine's coverage note is addressed in Adding the test as suggested (
The new parametrised test covers exactly the four unicode/CJK shapes from this file, which restores the coverage hiding One thing I did not do: the other benchmark modules still error without Re-requesting approval on |
There was a problem hiding this comment.
The #228 half of this PR is good and I verified it twice; the new commit's wider claim is where I have a problem.
Combining marks: fixed as described.
WIDE_CHARS_MODE=True, our _visible_width vs wcwidth.wcswidth
"cafe" + U+0301 ours 4 wcswidth 4 ok (was 5)
precomposed é ours 4 wcswidth 4 ok
CJK / fullwidth ours 4 wcswidth 4 ok
Devanagari virama now zero-width ok
Suites match yours: 94 passed on 3.10 and 3.12 with wcwidth present, 90 passed / 4 skipped without it.
What the new test does not cover. The docstring now says the flag "reproduces what the reference does with wcwidth", and test_wide_chars_mode_matches_the_wcwidth_reference parametrizes four shapes — combining-accent, CJK, mixed CJK/ASCII, one emoji. None of the four is a character outside unicodedata.combining()'s reach, so the test cannot fail on the rest of wcwidth's zero-width set. Measuring our _visible_width against wcswidth across that set:
ours wcswidth
emoji ZWJ family (👨👩👧) 8 2 MISMATCH
keycap 1️⃣ 3 2 MISMATCH
ZWSP in text 3 2 MISMATCH
ZWJ in text 3 1 MISMATCH
Devanagari vowel sign 5 4 MISMATCH
Hangul jamo (U+1100+U+1161) 3 2 MISMATCH
CGJ U+034F (ccc=0) 3 2 MISMATCH
enclosing keycap U+20E3 1 0 MISMATCH
mismatches: 8/18 shapes (the other 10 agree)
The cause is uniform: unicodedata.combining() is zero for format characters (U+200B/200D, U+FE0F), for Hangul medial/final jamo, for CGJ, and for enclosing marks — all of which wcwidth reports as 0. So the fix covers Mn/Me with a non-zero combining class and stops there.
Two ways to close this, both cheap, and I do not mind which:
- Widen the rule and narrow the claim. Zero-width for
category in {Mn, Me, Cf}plus the Hangul jamo medial/final ranges gets every mismatch above except the emoji ZWJ cluster, where wcwidth 0.9.3 counts a ZWJ sequence as one grapheme (2 columns for the family, ours 8). Then the docstring says "East Asian width and zero-width marks; ZWJ sequences are counted per component" instead of "what wcwidth reports". - Keep the implementation and narrow the claim to what is actually true today — combining marks and East Asian width — and say explicitly that ZWJ/VS/jamo sequences diverge.
What I would avoid is merging with the current wording: the test that was added to guarantee this path will pass forever while the flag stays visibly wrong for emoji, zero-width joiners and decomposed Hangul, and the next person to touch _visible_width will trust it.
Severity, for the record: latent for default users (flag is opt-in), live for anyone setting it — an emoji or ZWJ cell pads 6 columns short in the family case. Not a regression against master, which counts those as 1 as well; the PR improves it from 5 to 4 on combining marks and I would keep that part.
CI at the time of writing: lint and GitGuardian green, the four test legs still running.
There was a problem hiding this comment.
Re-approval on 19f1247. The bug is real, the fix is in the right place, and the default path is untouched - but the docstring now claims more than the code does, and I can show where.
The bug, isolated
Reverting only tabulate/tabulate.py to 9d34b8f and keeping the new test:
1 failed, 93 passed
FAILED test_wide_chars_mode_matches_the_wcwidth_reference[data0-kwargs0]
-> the cafe/eleve (decomposed) case; the CJK, mixed and emoji parameters pass
So the combining-mark case is exactly the one the old width calculation got wrong (café five columns instead of four), and the new parameter list pins it.
Default mode is unchanged
Old and new WIDE_CHARS_MODE=False rendering, 5 documents across 4 table formats: 0 differences of 20. WIDE_CHARS_MODE is still False by default, so nothing changes for a caller who never opts in.
Suites
wcwidth installed 94 passed
wcwidth absent 90 passed, 4 skipped (the four wcwidth parameters)
CI green on 3.10-3.13 plus lint, CLEAN.
Where unicodedata.combining() stops matching wcwidth
The new docstring says combining marks count as zero "which is what wcwidth reports", and the test is named ...matches_the_wcwidth_reference. unicodedata.combining(ch) is the canonical combining class, and it is 0 for plenty of zero-width characters, so six common shapes still diverge:
string ours wcswidth
👨👩👧 (ZWJ family) 8 2 <--
"ab" (ZWJ) 3 1 <--
"ab" (zero-width space) 3 2 <--
"가" (Hangul medial jamo) 3 2 <--
"a⃝" (enclosing combining) 2 1 <--
"a️b" (variation selector) 3 2 <--
precomposed/decomposed e-acute, multiple marks, CJK,
fullwidth, kana, emoji, Devanagari matra, Thai marks agree
(Control characters are a separate story: wcswidth returns -1 where we return a width, which is wcwidth's error convention rather than a width mismatch.)
All six have combining class 0, so the if unicodedata.combining(ch): continue branch never sees them. Either narrow the claim, or widen the test set:
if (unicodedata.combining(ch)
or ch in ""
or "︀" <= ch <= "️"
or "󠄀" <= ch <= "󠇯"
or "ᅠ" <= ch <= "ᇿ"): # Hangul medial/final jamo
continueThat covers the six above (ZWJ-joined emoji then measure 2 + 2 + 2 = 6 rather than 2, so full grapheme-cluster behaviour still needs wcwidth itself - worth saying so if the ambition is "matches"). Non-blocking: the reported bug is fixed, this is about the sentence that now promises parity.
There was a problem hiding this comment.
The fix is right and I would keep it; the claim it ships with is not, and that is why I am not approving this head.
What is verified. The reported bug is real and fixed: WIDE_CHARS_MODE=True, our _visible_width vs wcwidth.wcswidth:
'cafe' + U+0301 4 (was 5) wcswidth 4 ok
three stacked marks 1 wcswidth 1 ok
precomposed é unchanged wcswidth 4 ok
default (flag off) len(s) untouched ok
Suites: 94 passed on 3.11 with wcwidth 0.9.3; on 3.12 without it, 90 passed / 4 skipped with SKIPPED ... could not import 'wcwidth' at the new test. Teeth: replacing the unicodedata.combining(ch) check with if False: fails exactly test_wide_chars_mode_matches_the_wcwidth_reference[data0-kwargs0], the decomposed cafe\u0301 case, and nothing else. The test case really is decomposed, not precomposed - I checked the bytes in the file.
The documentation claim is false even on its narrow reading. unicodedata.combining() is not "is a combining mark"; it is "canonical combining class != 0". Four characters that are unambiguously combining marks have ccc = 0 and are counted as one column here while wcwidth counts zero:
U+034F COMBINING GRAPHEME JOINER Mn ccc=0 ours 1 wcwidth 0
U+20E3 COMBINING ENCLOSING KEYCAP Me ccc=0 ours 1 wcwidth 0
U+0488 COMBINING CYRILLIC HUNDRED THOU Me ccc=0 ours 1 wcwidth 0
U+FE0F VARIATION SELECTOR-16 Mn ccc=0 ours 1 wcwidth 0
so the docstring sentence "combining marks as zero, which is what wcwidth reports" does not hold for 1,063 Mn plus 13 Me code points.
Scale, over every code point. I compared _visible_width(ch) against wcwidth.wcwidth(ch) for all 1,112,064 non-surrogate code points with the flag on:
assigned code points that diverge: 2,160
Mn 1063 Mc 420 Lo 234 (Hangul jamo) So 191 Cf 149 Cc 65 Me 13 No 23 Zl 1
unassigned (Cn): 763,353 - these are why a naive ratio reads 68.84%; ignore them
The Cn mass is east_asian_width() returning W for unassigned code points, so it is noise rather than a table-rendering risk. The assigned rows are the real gap, and they are the classes Milo listed.
Practical strings that still render wrong with the flag on:
👨👩👧 (ZWJ family) ours 8 wcswidth 2
1️⃣ (keycap) ours 3 wcswidth 2
zero-width space ours 1 wcswidth 0
a + ZWJ + b ours 3 wcswidth 1
Hangul jamo U+1100+U+1161+U+11A8 ours 4 wcswidth 2
Hangul syllable + trailing jamo ours 3 wcswidth 2
What I would ask for, either is fine and both are cheap:
- Widen the rule to
unicodedata.category(ch) in {"Mn", "Me", "Cf"}plus the Hangul medial/final ranges (U+1160-U+11FF,U+D7B0-U+D7FF). That closes every row above except the ZWJ cluster, which needs grapheme segmentation (wcwidth 0.9.3 folds a ZWJ sequence into one grapheme). Then the docstring says "zero-width marks and joiners; ZWJ sequences count per component". - Keep the implementation and narrow the wording to "combining marks with a non-zero canonical combining class" plus an explicit sentence that format characters, jamo and ZWJ sequences are counted per code point.
The one thing I would not merge is the current pair of claim and test: four cases that can only fail for ccc > 0 marks, under a name that says the flag matches the wcwidth reference. The next person to touch _visible_width will believe it.
Severity is as Milo wrote - opt-in flag, not a regression against master, and the ccc > 0 fix is a strict improvement worth keeping. This is a wording/scope round, not a rework.
| return len(s) | ||
| w = 0 | ||
| for ch in s: | ||
| if unicodedata.combining(ch): |
There was a problem hiding this comment.
unicodedata.combining() returns the canonical combining class, which is 0 for a number of combining marks that wcwidth treats as zero-width - U+034F (Mn), U+20E3 and U+0488 (Me), U+FE0F (Mn). Measured: _visible_width("\u20e3") is 1 here and wcwidth.wcwidth("\u20e3") is 0, so a keycap emoji pads one column wide. unicodedata.category(ch) in {"Mn", "Me", "Cf"} covers those without pulling in a table; the Hangul medial/final ranges (U+1160-U+11FF, U+D7B0-U+D7FF) are the remaining Lo case, and only ZWJ clusters need more than a per-character rule.
Milo's review: unicodedata.combining() is zero for format characters, enclosing marks and Hangul jamo, so eight shapes still diverged from wcwidth while the docstring claimed parity. Zero-width now covers Mn/Me/Cf plus the Hangul medial/final jamo ranges, which matches wcwidth on every per-character shape measured; emoji presentation sequences (ZWJ clusters, keycaps) stay per-component and are documented as the one divergence. Tests pin both halves.
|
Milo's blocker is addressed in Widened: zero-width now covers The two remaining mismatches are the grapheme-cluster cases, so the docstring and the docs now say that explicitly instead of claiming "what Tests: eight per-character shapes compare Docs and changelog updated in both languages. Re-requesting approval on |
There was a problem hiding this comment.
Option 1 verified. The per-character table is clean now, the two cluster cases are documented rather than claimed as parity, and the tests pin both.
19f1247 644ba85
combining acute 4 / 4 4 / 4
Devanagari (U+0947 Mn) 5 / 4 4 / 4
ZWSP in text 3 / 2 2 / 2
CGJ U+034F 3 / 2 2 / 2
Hangul jamo U+1100+U+1161 3 / 2 2 / 2
enclosing keycap U+20E3 1 / 0 0 / 0
ZWJ family 8 / 2 6 / 2 documented, pinned
keycap 1️⃣ 3 / 2 1 / 2 documented, pinned
(ours / wcswidth; the last two are the grapheme-cluster cases the docstring now names.)
Suites match yours: 104 passed on 3.10 and 3.12 with wcwidth, 92 passed / 12 skipped without. CI green on 3.10-3.13, lint, GitGuardian.
One thing the widened rule changes that the docstring does not mention, non-blocking: Cf is not uniformly zero-width in wcwidth. Of the 170 Cf codepoints, 156 are zero there and 14 are not — and the new rule makes all 170 zero, so those 14 went from matching (ours=1, correct on the previous head) to diverging:
U+00AD soft hyphen wcwidth 1 ours 0
U+0600..U+0605 Arabic number signs wcwidth 1 ours 0
U+06DD, U+070F, U+0890, U+0891, U+08E2, U+110BD, U+110CD wcwidth 1 ours 0
So the sentence "format characters (Cf) ... as zero — the per-character widths wcwidth reports" is now false for those 14, and U+0600-U+0605 are Arabic text marks that appear in front of numerals, which is exactly where a column one short is visible. Two one-line options: add the 14 as an exception tuple next to _ZERO_WIDTH_RANGES, or soften the sentence to "format characters, except the prepended concatenation marks and the soft hyphen, which wcwidth counts as one".
It is a net improvement either way — 156 zero-width format characters fixed against 14 newly zero — and nothing in the tests I can write hangs on it, so I am approving. Mentioning it because it is the last remaining case where the comment and the behaviour disagree.
Thanks for taking the measurement the right way; the narrower claim plus the two pinned divergences is a much better contract than parity wording that only held for the four shapes it tested.
There was a problem hiding this comment.
Approved on 644ba85. Every per-character shape from my table now agrees, the two cluster cases are pinned instead of claimed, and the WIDE_CHARS_MODE docstring is finally honest about where it stops. One new disagreement came in with the wider rule; details below.
What I re-ran
wcwidth installed 104 passed
wcwidth absent 92 passed, 12 skipped (4 comparison + 8 zero-width parameters)
Whole-Unicode sweep, _visible_width(ch) vs wcwidth(ch) for every assigned code point:
combining acute / multiple marks / precomposed agree
ZWSP U+200B, ZWJ U+200D, RLM U+200F, BOM U+FEFF,
CGJ U+034F, Mongolian vowel separator U+180E,
word joiner U+2060 agree
Hangul U+1161 medial / U+11A8 final agree
CJK, fullwidth, kana, emoji agree
Cluster behaviour matches the new tests: ZWJ family 6, keycap 1, both documented as per-component.
The one thing the wider rule cost
Cf is not all zero-width. 14 of its members are width 1 under wcwidth, and the old rule (skip only combining marks) counted them correctly:
string 19f1247 644ba85 wcwidth
"ab" soft hyphen 3 2 3 <-- went from right to wrong
"ab" arabic number sign 3 2 3 <--
The full set, all ours=0 wcwidth=1: U+00AD, U+0600-U+0605, U+06DD, U+070F, U+0890, U+0891, U+08E2, U+110BD, U+110CD - soft hyphen plus the Arabic/Kaithi prepended concatenation marks. A soft hyphen in copied text is not exotic, and this is a regression against the previous head rather than a pre-existing gap:
_ZERO_WIDTH_CF = frozenset({
0x00AD, 0x0600, 0x0601, 0x0602, 0x0603, 0x0604, 0x0605,
0x06DD, 0x070F, 0x0890, 0x0891, 0x08E2, 0x110BD, 0x110CD,
})
def _is_zero_width(ch: str) -> bool:
cat = unicodedata.category(ch)
if cat in ("Mn", "Me"):
return True
if cat == "Cf":
return ord(ch) not in _ZERO_WIDTH_CF
...Residuals that are not this PR's business
For the record, after the fix the remaining differences against wcswidth are: unassigned code points (stdlib east_asian_width says wide for large Cn ranges, wcwidth says 1), a couple of hundred So/No characters where the two Unicode tables disagree on East Asian width (U+2630 trigrams, counting rods), Zl/Zp separators, and U+3164 HANGUL FILLER. Those predate this branch and come from the stdlib table being older than wcwidth's, so "matches wcwidth" will never be exactly true - which is why the docstring wording matters more than the last few code points.
CI green on 3.10-3.13 plus lint, CLEAN.
There was a problem hiding this comment.
This is the right shape, and the numbers back it up. Approved.
The rule now closes what it targets. Re-running my all-codepoint scan (flag on, _visible_width(ch) vs wcwidth.wcwidth(ch), 1,112,064 code points):
assigned code points that diverge
before the first fix (#228 half) 2,160
after 19f1247 (combining only) 2,160 (Mn 1063, Me 13, Cf 149, Mc 420, Lo 234, ...)
after 644ba85 742 (Mc 445, So 191, No 23, Cc 65, Cf 14, Lo 2, Zl 1, Zp 1)
and the 16-shape differential is down from 6 mismatches to 4, where the four are exactly the documented ones plus the two conventions (\x07 -> wcwidth -1, and a ZWJ inside plain text).
cafe+U+0301 4 = wcswidth ok ZWSP 0 = 0 ok
CGJ U+034F 0 = 0 ok Hangul jamo 2 = 2 ok
virama cluster 2 = 2 ok keycap 1 documented
precomposed 4 = 4 ok ZWJ family 6 documented
The divergence test is doing real work: test_emoji_presentation_sequences_count_per_component pins 6 and 1 without needing wcwidth, and its docstring states what wcwidth would say - so a future change to cluster handling fails here instead of silently drifting. The eight test_zero_width_shapes_match_wcwidth cases exercise the widened rule against wcswidth, and they skip cleanly without the package.
Suites match the description: 104 passed on 3.11 with wcwidth 0.9.3; on 3.12 without it, 92 passed / 12 skipped (four at line 77, eight at line 144).
One residue, not a merge blocker - I am filing it as #232 so it does not get lost:
Mc 445 codepoints ours 1-2 wcwidth 0 all of Devanagari/Bengali/Tamil vowel signs
Cf 14 codepoints ours 0 wcwidth 1 U+00AD soft hyphen, U+0600-U+0605, U+06DD, U+070F, ...
So 191 / No 23 ours 1 wcwidth 2 Unicode data drift between `unicodedata` and wcwidth's tables
Mc is worth a word, because it is the common case rather than an exotic one:
हिन्दी wcswidth 4 ours 5
বাংলা wcswidth 4 ours 5
rendered: reference zerodep
हिन्दी 1 हिन्दी 1
north 2 north 2
The ID column shifts by one on any row with a spacing vowel sign. Every one of the 445 Mc code points reports 0 under wcwidth - I checked, there is no Mc with width 1 - so adding "Mc" to _ZERO_WIDTH_CATEGORIES is exact for wcwidth parity, not an approximation. The So/No rows are a unicodedata version difference rather than a rule, and Cf over-zeroes those 14; both are better handled by documenting the residue than by adding exceptions.
That makes the docstring sentence "the per-character widths wcwidth reports" still slightly generous for Indic text, which is why the issue exists rather than a silent pass. The paragraph's explicit note about emoji presentation sequences is what made me comfortable leaving the merge to you.
| # enclosing marks (Me) — the keycap U+20E3 — and format characters (Cf), | ||
| # which include the zero-width space and joiner (U+200B, U+200D) and the | ||
| # combining grapheme joiner (U+034F). | ||
| _ZERO_WIDTH_CATEGORIES = frozenset({"Mn", "Me", "Cf"}) |
There was a problem hiding this comment.
Adding "Mc" here is exact rather than approximate, and it closes the one residue that hits ordinary text: every Mc code point reports 0 under wcwidth 0.9.3 (I checked all 445 - there is no Mc with width 1), while this rule leaves them at 1. Effect on real strings with the flag on:
हिन्दी wcswidth 4 ours 5
বাংলা wcswidth 4 ours 5
so the ID column shifts by one on any row containing an Indic spacing vowel sign. Cf deliberately stays as it is: the 14 exceptions (U+00AD, U+0600-U+0605, U+06DD, U+070F, U+110BD, U+110CD) are better documented than special-cased. Filed as #232 so it is tracked either way.
Review note on #229: hiding wcwidth made the default comparison environment-independent but dropped the only coverage of the wide-character width path. Adding it back exposed a real gap — WIDE_CHARS_MODE counted a combining accent as one column, so 'café' measured five where wcwidth reports four. Combining marks now contribute zero width, and a parametrised test compares WIDE_CHARS_MODE=True against the reference imported with wcwidth visible.
What
Fixes #228: four unicode/CJK comparisons in the tabulate correctness suite fail whenever
wcwidthis installed.Why
The reference
tabulatecomputes column widths withwcwidthwhen that package is importable and withlen(s)otherwise. Our module's default islen(s)(WIDE_CHARS_MODEis opt-in). The test module already hides the repo's owntabulate/directory from the reference import, but nothing hidwcwidth, so in an environment where it arrives as a transitive dependency the two sides applied different width rules:Measured:
wcwidthpresent → 4 failed, 79 passed; absent → 83 passed. CI does not installwcwidth, which is why this was green there.Changes
tabulate/test_tabulate_correctness.pyhideswcwidthfor the duration of the reference import (sys.modules["wcwidth"] = Nonemakes the import fail cleanly, so the reference selects itslen(s)path) and restores the previous state afterwards.tabulate.wcwidth is None, so the comparison cannot silently drift back to two different rules.Verification
Docs
Changelog entry under Infrastructure in
docs_enanddocs_zh.