Enhance nickname processing #122

aikimark · 2021-03-22T18:23:56Z

converted REGEXES from set() to list
added optional label/tag to regex tuples
created new self variable for nickname regexes
changed nickname processing logic
added tests for nicknames, including adding patterns and multiple nicknames

* changed regexes from set to list * added labels/tags as third item in tuples * changed processing of regexes input to TupleManager * added filtered list for nickname patterns * added/changed regexes

remove testREGEXES.py from repository

* test for adding nickname * test for multiple nicknames

* parse parenthesized suffixes (ret, vet) * converted unused suffix processing routine to preprocess the fullname ahead of nickname processing, since nickname patterns include a parenthesis-delimited pattern

derek73

I added a few comments but I think I missed pressing the final submit button. Sorry it has taken me so long to get to reviewing this again. Generally it looks good, I just have a few questions and there's a few and I'm not sure about adding Justice as a title because it precludes it being a first name.

derek73 · 2021-03-23T07:10:38Z

nameparser/config/__init__.py

@@ -231,7 +231,7 @@ def __init__(self,
        self.first_name_titles   = SetManager(first_name_titles)
        self.conjunctions        = SetManager(conjunctions)
        self.capitalization_exceptions = TupleManager(capitalization_exceptions)
-        self.regexes             = TupleManager(regexes)
+        self.regexes             = TupleManager([tpl[:2] for tpl in REGEXES])


I think this should use the local variable regexes to preserve the ability to pass it as an attribute to a new instance (not that anyone is doing that). ([tpl[:2] for tpl in regexes]).

What is the slice doing here? It's not clear to me.

I added a tag/label to some of the tuples. The slice returns the first two items in the tuples, omitting the tag/label data. The TupleManager object can still be used in the code. The regexes variable in the constants() is no longer a set object, just a list of tuples. I did this to preserve the order of the regex patterns.

Ok. But If someone tries to instantiate passing a TuperManger to regexes, it will have no effect because you are using the global variable instead of the local one. Need to replace REGEXES for regexes.

ex: name = HumanNam(regexes=myTupleManager) would fail. (I guess I should have some tests for those instantiation attributes.)

I don't understand. The list of compiled regex patterns for nicknames is different than the regexes that is fed into tuplemanager. I thought I'd left the tuplemanager-based regexes alone. I might have gotten a little confused by variables/functions with the same name. I'll take another look at it.

Some clarification would be helpful.

It's a simple mistake of using the module constant instead of the attribute passed to the class' init function.

change:
self.regexes = TupleManager([tpl[:2] for tpl in REGEXES])
to
self.regexes = TupleManager([tpl[:2] for tpl in regexes])

Here's a test that should pass but will fail with your code above.

def test_custom_regex_constant(self): t = {'test': 'test'} c = Constants(regexes=t) self.assertEqual(c.regexes, t)

(I edited the last line of my test to fix the equals test)

derek73 · 2021-03-23T07:14:07Z

nameparser/parser.py

@@ -243,7 +254,11 @@ def nickname(self):
        The person's nicknames. Any text found inside of quotes (``""``) or


This description of what constitutes a nickname should probably be updated.

Agreed. We've exchanged thoughts on this via email.

I added a couple of new nickname patterns and 'standardized' the existing patterns. Do you still have those emails?

derek73 · 2021-03-23T07:20:09Z

nameparser/parser.py

        # full_name setter triggers the parse
+        #========================================================
+        #IMPORTANT NOTE:
+        #  The followint statement must be the last one in the 


Should be "The following statement...", also could combine with the existing comment:

The following statement must be the last line in _init__ because it triggers the parse using :py:func:`full_name.setter`.

I agree. These two statements can be combined. I was 'bitten' by that when I started to change the code. I wanted to add text to really draw the attention of future collaborators.

derek73 · 2021-03-23T07:24:44Z

nameparser/config/suffixes.py

@@ -6,6 +6,7 @@
    'esq',
    'esquire',
    'jr',
+    'jr.',


some of these suffixes are abbreviations that could also end in a period, eg "esq". Some of them are not abbreviations, eg "iii". Do we need to enter period versions of all of them that are abbreviations?

I'd have to verify how you are processing the suffixes to respond with 100% certainty. Since the suffixes are in a list, I think the code is most likely doing a set inclusion test, using the in operator. Part of the answer depends on how you are parsing the text prior to doing the set inclusion test. If your parsing picks up periods, then my answer is "yes" - you will need to have both the plain and period-ending versions of the suffixes. Also, be aware that the set inclusion test is case sensitive.

In the new suffix preprocessing routine I added, the regex pattern has a ".?", which will match the prior text with and without a trailing period character. The question mark makes the period character optional.

What do you want to happen?

The parsing code does a split on space characters, so any trailing punctuation (comma, period, semicolon, etc.) will remain with the word.

the utility function lc() removes periods.

Looking at the is_suffix method in the parser,

# suffixes may have periods inside them like "M.D." return ((lc(piece).replace('.','') in self.C.suffix_acronyms) \ or (lc(piece) in self.C.suffix_not_acronyms)) \ and not self.is_an_initial(piece)

Looks like it was already a bit unclear before you started because .replace('.','') is unnecessary in that first line.

So, I think you should remove the "jr." entry because I don't expect it is making anything work. Probably there's something not quite right there but it's not related to any changes you made and we can fix it after merging your changes in if we want.

Is period removal sufficient? I'm thinking about other punctuation characters that may separate suffixes, such as comma.

The first step of the parser is to split the entire string on commas. Many suffixes listed at the end of a name are always treated as suffixes. These are the 2 supported formats for commas:

Lastname [Suffix], Title Firstname (Nickname) Middle Middle[,] Suffix [, Suffix]
Title Firstname M Lastname [Suffix], Suffix [Suffix] [, Suffix]

derek73 · 2021-03-23T07:29:50Z

nameparser/config/suffixes.py

@@ -25,6 +26,7 @@
 """
 SUFFIX_ACRONYMS = set([
    '(ret)',
+    '(ret.)',


The SUFFIX_ACRONYMS set was intended to be acronyms that could optionally be separated by periods. I'm not sure how (ret) got in there because it's surprising to see parenthesis in there, but it doesn't seem to make sense to add a period to one of the items in this set.

The "(ret)" was there when I started coding. I didn't know how you might want to parse the "ret" and "vet" suffixes, so I tried to keep the parentheses in the parsed result. I think I added trailing-period versions to the list before I realized that any parenthesis-delimited string would be parsed by the nickname routine. I can certainly change how I'm processing these two as well as add other suffix strings to the preprocessing pattern. Just let me know.

What do you want to happen with ret and vet?

Does something fail if you remove the one with the period?

No. I'm using a regex pattern for the (ret) and (vet) suffixes. The regex pattern ignores any trailing period. Using the existing list of suffixes doesn't work, since the nickname processing picks these up, matching the parenthesis pattern. Nickname processing happens before suffix processing, which is why I added a suffix preprocessor routine for these two parenthesis-delimited suffixes.

I can retain the parentheses or drop them. Let me know and I'll make sure the code does what you want, relative to these two suffixes.

If you have changed the way that suffixes are detected then I think you need to modify the is_suffix method to implement that test using your new regexes.

derek73 · 2021-03-23T07:31:54Z

nameparser/config/titles.py

@@ -166,6 +166,7 @@
    'chef',
    'chemist',
    'chief',
+    'chief justice',


I'm kind of surprised if this works, but I guess I could see it because of how the titles chain together. "Justice" is a somewhat common first name so we couldn't just add that as it's own title, so if this works it's a nice workaround.

I didn't realize that "Justice" was a common first name. I don't think my "Chief Justice" string is being matched due to the prior parsing actions. I'm not sure what qualifies as "common first name". "Justice" is around the 580th most common first name in America. However, I think it is probably more common than the number of judges.

I spoke with a judge and asked him about the use of the title "Justice". He said it was rare. I'll undo this change, since it was based on a false assumption.

The judge expressed some dismay that titles were being used as first name. He has encountered people with first names, such as "King" and "Queen", in his courtroom.

We might want to include the parser's bias for first names over titles.

If someone is parsing names of titled people (think UN delegations), what should they do?

A bias for first names over titles is already a feature of the parser, and why there are no potential first names in the titles constant. First job of a name parser is to parse names, then it can optionally parse titles but not if it messes up names.

I'll do some testing after removing "Justice" and "Chief Justice" from the list. I might add a tests for "David Justice" and "Justice, David", the baseball player.

I work in legal data, and I'll note that there are very few "Justices" though I suppose "justice of the peace" is a title. But in my experience, "justice" is reserved pretty much exclusively for the SCOTUS justices. You can see the way this shakes out on this page (though it doesn't discuss this topic): https://www.uscourts.gov/judges-judgeships/about-federal-judges

Oh, sorry, meant to say that relatedly, "J." is a very common title among judges. I'm guessing it can't be added b/c it's one letter, but I thought I'd throw that out there.

tests.py

derek73 · 2021-03-23T07:35:36Z

tests.py

@@ -1766,6 +1796,21 @@ def test_suffix_with_periods_with_lastname_comma(self):
        self.m(hn.last, "Doe", hn)
        self.m(hn.suffix, "Msc.Ed.", hn)

+    @unittest.SkipTest
+    def test_suffix_in_nickname_dup(self):


would be good to have comments about what these skipped tests are doing here

I'll look at this. I might have added these tests for my initial approach to the problem. Once I changed my approach, I just skipped the test rather than delete the (test_) function. Looking at the code will jog my memory.

I'm going to ask a retired judge about the titles that one might apply to his profession. I'll email you after we've talked.

derek73 · 2021-03-23T07:39:35Z

nameparser/config/titles.py

@@ -339,6 +340,7 @@
    'judicial',
    'junior',
    'jurist',
+    'justice',


My friend Justice would be upset that the parser would not recognize his first name.

There are about the same number of people named "Justice" as there are judges in America (~33000). What does the name parser do, or what should it do, when it encounters several names? What if "Justice" is one of the first of the words in a multi-name string?

This is a question similar to the one that I posed for myself when I first approached the problem of nicknames that might also be suffixes. I didn't have a good answer, so I abandoned that original approach. It is still an unanswered question.

The simplest use case for this parser is just Firstname Lastname. I feel like when there is conflict with other things the parser should/could do (ex: recognize titles), those other things should be sacrificed to preserve it's ability to split up a simple name. There is a fairly simple workaround if someone using the parser wants to change it, and a human interacting with the parser could add their fist name and the parser would then figure out that it's a title, kind of like if you were interacting with a human and they had the same confusion.

I hadn't noticed a test for this. I'll look at it and alter my John Roberts test accordingly.

I'll remove "Justice" from the titles list.

aikimark · 2021-05-05T01:36:55Z

I have responded via email.

Should I copy my response into the textboxes above? This is my first pull request.

derek73 · 2021-05-05T18:38:04Z

I think if you respond in the github UI it just facilitates the conversations about comment. I will take a closer look at your email later tonight. - Derek Gulbranson

…

On May 4, 2021, at 6:37 PM, aikimark ***@***.***> wrote: I have responded via email. Should I copy my response into the textboxes above? This is my first pull request. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or unsubscribe.

derek73

I think that you have to make any changes to this pull request in your clone of the project and then push them to this change request somehow.

derek73 · 2021-05-07T03:51:31Z

nameparser/config/__init__.py

@@ -231,7 +231,7 @@ def __init__(self,
        self.first_name_titles   = SetManager(first_name_titles)
        self.conjunctions        = SetManager(conjunctions)
        self.capitalization_exceptions = TupleManager(capitalization_exceptions)
-        self.regexes             = TupleManager(regexes)
+        self.regexes             = TupleManager([tpl[:2] for tpl in REGEXES])


Ok. But If someone tries to instantiate passing a TuperManger to regexes, it will have no effect because you are using the global variable instead of the local one. Need to replace REGEXES for regexes.

ex: name = HumanNam(regexes=myTupleManager) would fail. (I guess I should have some tests for those instantiation attributes.)

aikimark added 4 commits March 20, 2021 11:36

regex changes

6eb66ba

* changed regexes from set to list * added labels/tags as third item in tuples * changed processing of regexes input to TupleManager * added filtered list for nickname patterns * added/changed regexes

remove testREGEXES

4a395cf

remove testREGEXES.py from repository

added nickname tests

502335d

* test for adding nickname * test for multiple nicknames

added suffix preprocessing

129667c

* parse parenthesized suffixes (ret, vet) * converted unused suffix processing routine to preprocess the fullname ahead of nickname processing, since nickname patterns include a parenthesis-delimited pattern

derek73 requested changes Apr 28, 2021

View reviewed changes

derek73 requested changes May 7, 2021

View reviewed changes

derek73 added the enhancement label May 21, 2021

		@@ -243,7 +254,11 @@ def nickname(self):
		The person's nicknames. Any text found inside of quotes (``""``) or

Enhance nickname processing #122

Are you sure you want to change the base?

Enhance nickname processing #122

Conversation

aikimark commented Mar 22, 2021

derek73 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derek73 May 18, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aikimark commented May 5, 2021

derek73 commented May 5, 2021 via email

derek73 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

derek73 May 18, 2021 •

edited