Fix leading space in surnames after capitalize() with empty middle name#164
Open
patchwright wants to merge 1 commit into
Open
Fix leading space in surnames after capitalize() with empty middle name#164patchwright wants to merge 1 commit into
patchwright wants to merge 1 commit into
Conversation
capitalize() split each attribute with str.split(' '), which returns ['']
(not []) for an empty string. cap_piece() returns '' for an empty part, so an
empty middle name produced middle_list = [''], which leaked into surnames_list
(middle_list + last_list) and yielded a leading space in the surnames property:
>>> hn = HumanName('john doe'); hn.capitalize(); hn.surnames
' Doe' # leading space (should be 'Doe')
The same spurious '' element also appeared in title_list/first_list/last_list
for empty attributes. Using str.split() instead returns [] for empty strings
and is equivalent for the already-whitespace-collapsed pieces cap_piece()
returns. The suffix split (', ') is intentionally left unchanged.
Added a regression test in HumanNameCapitalizationTestCase.
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.
Summary
After
capitalize()is called on a name that has no middle name (or any other empty attribute),surnamescarries a spurious leading space and the*_listattributes become['']instead of[].Reproduction
The leading space is a real observable artifact, not just an empty-list detail: downstream code that joins or compares surnames silently breaks.
Cause
cap_piece(...).split(' ')usesstr.split(' ')with an explicit separator. On an empty string,split(' ')returns[''](it preserves the empty token), whereasstr.split()with no argument collapses runs of whitespace and returns[]for an empty/whitespace-only string — which is the correct result for a list of name tokens.Fix
Drop the explicit separator for the whitespace-delimited attributes:
The
suffixline is intentionally left as.split(', '), because commas are a deliberate delimiter there.Tests
Adds
test_capitalize_empty_name_part_has_no_leading_space_in_surnames, assertingsurnames == 'Doe',middle_list == [], andsurnames_list == ['Doe']aftercapitalize()(covers theforce=Truepath too). The case fails on the current code and passes with the fix. Full suite: 345 tests OK (skipped=2, expected failures=10).I couldn't find an existing issue or PR covering this — happy to fold it into one if I missed it.