Fix bugs hidden in untested parser paths; remove vestigial unparsable#216
Merged
Conversation
Auditing the 11 uncovered lines in parser.py (98% coverage) turned up
three real bugs, dead code, and a few untested-but-live paths:
- Fix __hash__ to lowercase like __eq__ does, so equal HumanName
instances hash equal and behave correctly in sets and dicts
- Fix initials() emitting a stray empty initial ("J. . V.") — or
raising TypeError when empty_attribute_default is None — for name
parts with no initialable words (e.g. prefix-only middle "de la");
deduplicate the per-group comprehensions into _initials_lists()
- Fix a trailing suffix being silently dropped after an empty comma
segment, e.g. "Doe, John,, Jr." losing the "Jr."
- Remove the unparsable attribute: the len(self) < 0 guard meant to
set it was unreachable, so it has reported False for every parsed
name since the earliest releases
- Remove __ne__ (Python 3 derives != from __eq__) and the two
unreachable "if not nxt" branches in the parse loops, which the
vacuously-true are_suffixes() check always preempts
- Rename __process_initial__ to _process_initial: dunder-both-sides
names are reserved for Python special methods
Adds dunder-contract, initials, and suffix regression tests; parser.py
is now at 100% statement coverage.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t empty parse
- Correct release log: the unparsable guard broke in 2013 (v0.2.9), not
in the earliest releases (2011-2013 releases had a working attribute)
- Pin multiple empty comma segments ("Doe, John,, Jr.,, III") so a
break-instead-of-continue regression can't pass the single-empty test
- Pin hash interop with plain strings ("john smith" in {HumanName(...)}),
which depends on hashing str(self).lower() specifically
- Tighten the "final piece" comments in parse_full_name and document
are_suffixes()'s vacuous-truth contract that the piece loops rely on
- Document in usage.rst that empty/garbage input parses to an all-empty
name, checked via len(name) == 0
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tead Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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
1 participant
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 changed
A coverage audit of the 11 uncovered lines in
parser.py(98% at the time) found three real bugs, several lines of dead code, and a few untested-but-live paths. This PR fixes all of them;parser.pyis now at 100% statement coverage (1298 tests, up from 1282).Bug fixes
__hash__violated the hash/eq contract:__eq__compares lowercased strings but__hash__didn't lowercase, soHumanName("John Smith") == HumanName("john smith")wasTruewhile their hashes differed —{hn1, hn2}had size 2. Now hashes the lowercased string.initials()malformed output / crash: a name part with no initialable words (e.g. prefix-only middle name"de la"in"Vega, Juan de la") produced"J. . V."— and raisedTypeErrorwhenempty_attribute_defaultisNone. Such parts are now dropped; the duplicated per-group comprehensions ininitials()/initials_list()were deduplicated into a_initials_lists()helper."Doe, John,, Jr."lost theJr.because the oldif parts[2]:guard skipped all remaining segments when the first was empty. Empty segments are now skipped individually.Removals (breaking)
unparsableattribute removed: theif len(self) < 0:guard meant to set it was unreachable (__len__can't be negative), so it has reportedFalsefor every parsed name — including empty input — since the earliest releases.__ne__removed: Python 3 derives!=from__eq__automatically.__process_initial__renamed to_process_initial: double-underscore-both-sides names are reserved for Python special methods. Subclasses overriding the old name must rename their override.if not nxt:branches removed from the parse loops: the final piece always hits theare_suffixes(pieces[i+1:])branch first becauseall()over the empty tail is vacuouslyTrue. Comments now document that behavior at both sites.Notes for review
unparsableremoval pin); documenting tests were added for!=,__setitem__KeyError,repr, and thestring_format=Nonefallback in__str__.assertFalse(hn.unparsable)assertions were removed from existing tests.break(same vacuous-truth reason), and_process_initial's list guard can't be false forstr.split()output.🤖 Generated with Claude Code