Merge pull request #216 from derek73/fix/parser-coverage-gaps

Fix bugs hidden in untested parser paths; remove vestigial unparsable

Author: derek73

docs/release_log.rst

@@ -1,6 +1,12 @@
 Release Log
 ===========
 * 1.3.0 - Unreleased
+    - Remove the vestigial ``unparsable`` attribute: the guard that was meant to set it has been unreachable since 2013 (v0.2.9), so it has reported ``False`` for every parsed name for over a decade; check ``len(name) == 0`` to detect an empty parse
+    - Fix ``__hash__`` to lowercase the name like ``__eq__`` does, so equal ``HumanName`` instances hash equal and behave correctly in sets and dicts
+    - Fix ``initials()`` emitting a stray empty initial (e.g. ``"J. . V."``) -- or raising ``TypeError`` when ``empty_attribute_default`` is ``None`` -- for name parts with no initialable words, e.g. a prefix-only middle name like ``"de la"``
+    - Fix a trailing suffix being silently dropped after an empty comma segment, e.g. ``"Doe, John,, Jr."`` losing the ``"Jr."``
+    - Remove ``__ne__``; Python 3 derives ``!=`` from ``__eq__`` automatically
+    - Change internal initials helper ``__process_initial__`` to ``_process_initial``: double-underscore-both-sides names are reserved for Python special methods; subclasses overriding the old name must rename their override
     - Add ``non_first_name_prefixes`` to ``Constants``: a leading particle that is never a first name (e.g. ``"de Mesnil"``, ``"dos Santos"``) now parses as a surname with an empty first name, instead of treating the particle as the first name (closes #121)
     - Add a first-class ``maiden`` field and ``maiden_delimiters`` to ``Constants``, so a delimiter (e.g. parenthesis) can be routed to ``maiden`` instead of ``nickname`` for alternate/maiden surnames, e.g. ``"Baker (Johnson), Jenny"`` (closes #22)
     - Fix suffix-shaped parenthesized/quoted content (e.g. ``"(Ret)"``, ``"(MBA)"``) being misclassified as a nickname instead of a suffix (closes #111)

docs/usage.rst

@@ -74,6 +74,10 @@ Requires Python 3.10+.
     >>> name[1:-3]
     ['Juan', 'Q. Xavier', 'de la Vega']
 
+Empty or unparsable input does not raise an error; it produces a name whose
+attributes are all empty. Check ``len(name) == 0`` (or ``str(name) == ''``)
+to detect that nothing was parsed.
+
 
 Capitalization Support
 ----------------------

nameparser/parser.py

@@ -82,7 +82,6 @@ class HumanName:
 
     _count = 0
     _members = ['title', 'first', 'middle', 'last', 'suffix', 'nickname', 'maiden']
-    unparsable = True
     _full_name = ''
 
     title_list: list[str]
@@ -131,7 +130,6 @@ def __init__(
             self.suffix = suffix
             self.nickname = nickname
             self.maiden = maiden
-            self.unparsable = False
         else:
             # full_name setter triggers the parse
             self.full_name = full_name
@@ -163,9 +161,6 @@ def __eq__(self, other: object) -> bool:
         """
         return str(self).lower() == str(other).lower()
 
-    def __ne__(self, other: object) -> bool:
-        return not str(self).lower() == str(other).lower()
-
     @overload
     def __getitem__(self, key: slice) -> list[str]: ...
     @overload
@@ -202,23 +197,21 @@ def __str__(self) -> str:
         return " ".join(self)
 
     def __hash__(self) -> int:
-        return hash(str(self))
+        # __eq__ compares lowercased strings, so hash the lowercased string
+        # to keep equal instances in the same hash bucket.
+        return hash(str(self).lower())
 
     def __repr__(self) -> str:
-        if self.unparsable:
-            _string = f"<{self.__class__.__name__} : [ Unparsable ] >"
-        else:
-            attrs = (
-                f"    title: {self.title or ''!r}\n"
-                f"    first: {self.first or ''!r}\n"
-                f"    middle: {self.middle or ''!r}\n"
-                f"    last: {self.last or ''!r}\n"
-                f"    suffix: {self.suffix or ''!r}\n"
-                f"    nickname: {self.nickname or ''!r}\n"
-                f"    maiden: {self.maiden or ''!r}"
-            )
-            _string = f"<{self.__class__.__name__} : [\n{attrs}\n]>"
-        return _string
+        attrs = (
+            f"    title: {self.title or ''!r}\n"
+            f"    first: {self.first or ''!r}\n"
+            f"    middle: {self.middle or ''!r}\n"
+            f"    last: {self.last or ''!r}\n"
+            f"    suffix: {self.suffix or ''!r}\n"
+            f"    nickname: {self.nickname or ''!r}\n"
+            f"    maiden: {self.maiden or ''!r}"
+        )
+        return f"<{self.__class__.__name__} : [\n{attrs}\n]>"
 
     def as_dict(self, include_empty: bool = True) -> dict[str, str]:
         """
@@ -246,7 +239,7 @@ def as_dict(self, include_empty: bool = True) -> dict[str, str]:
                     d[m] = val
         return d
 
-    def __process_initial__(self, name_part: str, firstname: bool = False) -> str:
+    def _process_initial(self, name_part: str, firstname: bool = False) -> str:
         """
             Name parts may include prefixes or conjunctions. This function filters these from the name unless it is
             a first name, since first names cannot be conjunctions or prefixes.
@@ -259,8 +252,21 @@ def __process_initial__(self, name_part: str, firstname: bool = False) -> str:
                     initials.append(part[0])
         if len(initials) > 0:
             return self.initials_separator.join(initials)
-        else:
-            return self.C.empty_attribute_default
+        # Return '' (never empty_attribute_default, which may be None) when a
+        # part has no initialable words, e.g. a middle name consisting only of
+        # prefixes ("de la"). Callers drop these parts entirely.
+        return ''
+
+    def _initials_lists(self) -> tuple[list[str], list[str], list[str]]:
+        """Initials for the first, middle and last name groups. Parts that
+        yield no initials (e.g. a prefix-only middle name like "de la") are
+        dropped rather than kept as empty strings.
+        """
+        def group_initials(names: list[str], firstname: bool = False) -> list[str]:
+            return [i for i in (self._process_initial(n, firstname) for n in names if n) if i]
+        return (group_initials(self.first_list, True),
+                group_initials(self.middle_list),
+                group_initials(self.last_list))
 
     def initials_list(self) -> list[str]:
         """
@@ -275,9 +281,7 @@ def initials_list(self) -> list[str]:
                 >>> name.initials_list()
                 ['J', 'D']
         """
-        first_initials_list = [self.__process_initial__(name, True) for name in self.first_list if name]
-        middle_initials_list = [self.__process_initial__(name) for name in self.middle_list if name]
-        last_initials_list = [self.__process_initial__(name) for name in self.last_list if name]
+        first_initials_list, middle_initials_list, last_initials_list = self._initials_lists()
         return first_initials_list + middle_initials_list + last_initials_list
 
     def initials(self) -> str:
@@ -303,14 +307,13 @@ def initials(self) -> str:
             'J A D'
         """
 
-        first_initials_list = [self.__process_initial__(name, True) for name in self.first_list if name]
-        middle_initials_list = [self.__process_initial__(name) for name in self.middle_list if name]
-        last_initials_list = [self.__process_initial__(name) for name in self.last_list if name]
+        first_initials_list, middle_initials_list, last_initials_list = self._initials_lists()
 
-        # Empty parts must render as '' (not empty_attribute_default, which may be
-        # None) so str.format does not interpolate the literal "None" into the
-        # output. A fully-empty result falls back to empty_attribute_default,
-        # matching the other attribute accessors (e.g. ``first``).
+        # Empty name groups must render as '' (not empty_attribute_default,
+        # which may be None) so str.format does not interpolate the literal
+        # "None" into the output. A fully-empty result falls back to
+        # empty_attribute_default, matching the other attribute accessors
+        # (e.g. ``first``).
         initials_dict = {
             "first":  (self.initials_delimiter + self.initials_separator).join(first_initials_list) + self.initials_delimiter
             if len(first_initials_list) else "",
@@ -648,7 +651,12 @@ def is_suffix(self, piece: str) -> bool:
                 and not self.is_an_initial(piece)
 
     def are_suffixes(self, pieces: Iterable[str]) -> bool:
-        """Return True if all pieces are suffixes."""
+        """Return True if all pieces are suffixes.
+
+        Vacuously True for an empty iterable — the piece loops in
+        :py:func:`parse_full_name` rely on this to route the final piece
+        to the last-name branch.
+        """
         for piece in pieces:
             if not self.is_suffix(piece):
                 return False
@@ -996,7 +1004,6 @@ def parse_full_name(self) -> None:
         self.suffix_list = []
         self.nickname_list = []
         self.maiden_list = []
-        self.unparsable = True
 
         self.pre_process()
 
@@ -1043,12 +1050,13 @@ def parse_full_name(self) -> None:
                             self.is_roman_numeral(nxt) and i == p_len - 2
                             and not self.is_an_initial(piece)
                 ):
+                    # any piece reaching this check as the final piece lands
+                    # here: are_suffixes() is vacuously True for the empty
+                    # tail, making this the last-name branch as well as the
+                    # suffix branch
                     self.last_list.append(piece)
                     self.suffix_list += pieces[i+1:]
                     break
-                if not nxt:
-                    self.last_list.append(piece)
-                    continue
 
                 self.middle_list.append(piece)
         else:
@@ -1092,12 +1100,12 @@ def parse_full_name(self) -> None:
                         self.first_list.append(piece)
                         continue
                     if self.are_suffixes(pieces[i+1:]):
+                        # the final piece always lands here: are_suffixes() is
+                        # vacuously True for the empty tail, making this the
+                        # last-name branch as well as the suffix branch
                         self.last_list.append(piece)
                         self.suffix_list = pieces[i+1:] + self.suffix_list
                         break
-                    if not nxt:
-                        self.last_list.append(piece)
-                        continue
                     self.middle_list.append(piece)
             else:
 
@@ -1145,17 +1153,12 @@ def parse_full_name(self) -> None:
                         self.suffix_list.append(piece)
                         continue
                     self.middle_list.append(piece)
-                try:
-                    if parts[2]:
-                        for part in parts[2:]:
-                            self.suffix_list += self.expand_suffix_delimiter(part)
-                except IndexError:
-                    pass
+                for part in parts[2:]:
+                    # skip empty segments from doubled commas ("Doe, John,, Jr.")
+                    # without dropping the segments that follow them
+                    if part:
+                        self.suffix_list += self.expand_suffix_delimiter(part)
 
-        if len(self) < 0:
-            log.info("Unparsable: \"%s\" ", self.original)
-        else:
-            self.unparsable = False
         self.post_process()
 
     def parse_pieces(self, parts: Iterable[str], additional_parts_count: int = 0) -> list[str]:

tests/test_initials.py

@@ -37,6 +37,16 @@ def test_initials_all_empty_returns_empty_attribute_default(self) -> None:
         hn.C.empty_attribute_default = None
         self.assertEqual(hn.initials(), None)
 
+    def test_initials_middle_name_all_prefixes(self) -> None:
+        # "Vega, Juan de la" parses with middle name "de la", which contains
+        # no initialable words (both are prefixes). The part must be skipped
+        # entirely — not emit an empty initial ("J. . V.") and not crash when
+        # empty_attribute_default is None.
+        hn = HumanName("Vega, Juan de la")
+        self.m(hn.middle, "de la", hn)
+        self.assertEqual(hn.initials_list(), ["J", "V"])
+        self.assertEqual(hn.initials(), "J. V.")
+
     def test_initials_complex_name(self) -> None:
         hn = HumanName("Doe, John A. Kenneth, Jr.")
         self.m(hn.initials(), "J. A. K. D.", hn)
@@ -111,11 +121,11 @@ def test_initials_separator_kwarg(self) -> None:
         self.m(hn.initials(), "J.A.K.D.", hn)
 
     def test_initials_separator_custom_value(self) -> None:
-        # Non-empty custom separator exercising __process_initial__ on a multi-word
+        # Non-empty custom separator exercising _process_initial on a multi-word
         # token. "Van Berg" is a single name part whose two words produce two initials
         # joined by initials_separator.
         hn = HumanName("", initials_separator="-", initials_delimiter=".")
-        result = hn.__process_initial__("Van Berg", firstname=True)
+        result = hn._process_initial("Van Berg", firstname=True)
         self.assertEqual(result, "V-B")
 
     def test_str_default_behavior_unchanged(self) -> None:
@@ -126,46 +136,39 @@ def test_str_default_behavior_unchanged(self) -> None:
 
     def test_constructor_first(self) -> None:
         hn = HumanName(first="TheName")
-        self.assertFalse(hn.unparsable)
         self.m(hn.first, "TheName", hn)
 
     def test_constructor_middle(self) -> None:
         hn = HumanName(middle="TheName")
-        self.assertFalse(hn.unparsable)
         self.m(hn.middle, "TheName", hn)
 
     def test_constructor_last(self) -> None:
         hn = HumanName(last="TheName")
-        self.assertFalse(hn.unparsable)
         self.m(hn.last, "TheName", hn)
 
     def test_constructor_title(self) -> None:
         hn = HumanName(title="TheName")
-        self.assertFalse(hn.unparsable)
         self.m(hn.title, "TheName", hn)
 
     def test_constructor_suffix(self) -> None:
         hn = HumanName(suffix="TheName")
-        self.assertFalse(hn.unparsable)
         self.m(hn.suffix, "TheName", hn)
 
     def test_constructor_nickname(self) -> None:
         hn = HumanName(nickname="TheName")
-        self.assertFalse(hn.unparsable)
         self.m(hn.nickname, "TheName", hn)
 
     def test_constructor_multiple(self) -> None:
         hn = HumanName(first="TheName", last="lastname", title="mytitle", full_name="donotparse")
-        self.assertFalse(hn.unparsable)
         self.m(hn.first, "TheName", hn)
         self.m(hn.last, "lastname", hn)
         self.m(hn.title, "mytitle", hn)
 
     def test_initials_separator_kwarg_multiword_part(self) -> None:
-        # Regression: initials_separator kwarg must flow into __process_initial__
+        # Regression: initials_separator kwarg must flow into _process_initial
         # for multi-word name parts, not just into the initials() join calls.
         hn = HumanName("", initials_separator="")
-        result = hn.__process_initial__("Van Berg", firstname=True)
+        result = hn._process_initial("Van Berg", firstname=True)
         self.assertEqual(result, "VB")
 
     def test_string_format_empty_string_kwarg(self) -> None:

tests/test_nicknames.py

@@ -232,7 +232,6 @@ def test_maiden_via_constructor_kwarg(self) -> None:
         self.m(hn.first, "Jenny", hn)
         self.m(hn.last, "Baker", hn)
         self.m(hn.maiden, "Johnson", hn)
-        self.assertFalse(hn.unparsable)
 
     def test_maiden_name_in_parenthesis_with_comma(self) -> None:
         C = Constants()

tests/test_prefixes.py

@@ -87,7 +87,6 @@ def test_many_repeated_prefixes_does_not_blow_up(self) -> None:
         # regression has been reintroduced.
         name = "Jan " + "van der " * 30 + "Berg"
         hn = HumanName(name)
-        self.assertFalse(hn.unparsable)
         self.m(hn.first, "Jan", hn)
         self.assertIn("Berg", hn.last)
 

tests/test_python_api.py

@@ -36,6 +36,9 @@ def test_len(self) -> None:
         self.m(len(hn), 5, hn)
         hn = HumanName("John Doe")
         self.m(len(hn), 2, hn)
+        # empty input parses to an all-empty name; len == 0 is the
+        # documented emptiness check (see usage.rst)
+        self.assertEqual(len(HumanName("")), 0)
 
     @pytest.mark.skipif(not dill, reason="requires python-dill module to test pickling")
     def test_config_pickle(self) -> None:
@@ -215,6 +218,42 @@ def test_comparison_case_insensitive(self) -> None:
         self.assertIsNot(hn1, hn2)
         self.assertTrue(hn1 == "Dr. John P. Doe-ray clu, CFP, LUTC")
 
+    def test_hash_matches_case_insensitive_equality(self) -> None:
+        # __eq__ compares lowercased strings, so __hash__ must too:
+        # equal objects are required to have equal hashes.
+        hn1 = HumanName("Doe-Ray, Dr. John P., CLU, CFP, LUTC")
+        hn2 = HumanName("dr. john p. doe-Ray, CLU, CFP, LUTC")
+        self.assertEqual(hn1, hn2)
+        self.assertEqual(hash(hn1), hash(hn2))
+        self.assertEqual(len({hn1, hn2}), 1)
+        # __eq__ also accepts plain strings, so hashing str(self).lower()
+        # specifically (not e.g. an attribute tuple) is what lets strings and
+        # HumanName instances interoperate in sets and dicts
+        hn = HumanName("John Smith")
+        self.assertEqual(hash(hn), hash("john smith"))
+        self.assertIn("john smith", {hn})
+
+    def test_not_equal_operator(self) -> None:
+        self.assertTrue(HumanName("John Smith") != HumanName("Jane Smith"))
+        self.assertFalse(HumanName("John Smith") != HumanName("john smith"))
+
+    def test_unparsable_attribute_removed(self) -> None:
+        # Removed in 1.3.0: the guard that reported unparsable names was
+        # unreachable, so the attribute was always False after any parse.
+        self.assertFalse(hasattr(HumanName("John Smith"), "unparsable"))
+        self.assertFalse(hasattr(HumanName(first="John"), "unparsable"))
+
+    def test_str_fallback_without_string_format(self) -> None:
+        # string_format=None falls back to joining the non-empty attributes
+        hn = HumanName("Dr. John A. Doe, Jr.")
+        hn.string_format = None
+        self.assertEqual(str(hn), "Dr. John A. Doe Jr.")
+
+    def test_repr_blank_name(self) -> None:
+        hn = HumanName()
+        self.assertIn("first: ''", repr(hn))
+        self.assertIn(hn.__class__.__name__, repr(hn))
+
     def test_slice(self) -> None:
         hn = HumanName("Doe-Ray, Dr. John P., CLU, CFP, LUTC")
         self.m(list(hn), ['Dr.', 'John', 'P.', 'Doe-Ray', 'CLU, CFP, LUTC'], hn)
@@ -240,6 +279,11 @@ def test_setitem(self) -> None:
         with pytest.raises(TypeError):
             hn["suffix"] = {"test": "test"}
 
+    def test_setitem_invalid_key_raises_keyerror(self) -> None:
+        hn = HumanName("Dr. John A. Kenneth Doe, Jr.")
+        with pytest.raises(KeyError):
+            hn["bogus"] = "value"
+
     def test_conjunction_names(self) -> None:
         hn = HumanName("johnny y")
         self.m(hn.first, "johnny", hn)