Library: Should Person.merge_with_explicit() support merge of two verified Persons?

[nschneid]

#7599 presents such a case. There is an old verified Person without ORCID, and a newer one with ORCID, and they are really the same individual. I want to merge in a way that keeps the simpler (older) person ID but copies the ORCID from the newer Person. @mbollmann

[mbollmann]

We could:

• Change .merge_with_explicit() to .merge_into(), preserving current functionality for unverified Persons, but adding support for verified Persons too.
• Change .make_explicit() to .set_explicit_id(), preserving current functionality for unverified Persons, but adding the functionality of setting IDs on all papers of a verified Person too, as you recently requested in another thread.

That would make both of these functions more flexible. I could probably add this onto #7570.

[nschneid]

The first one sounds good! My concern with the second change is that the parameters would be different for the two scenarios: if the Person is already verified, there is no need to pass the ID (and in fact it would be confusing if a new ID was allowed).

How about:

• .merge_into(person)
• .make_id_explicit(newid=None, suffix=None, specify_on_items=True) (Precondition: unverified. By default, infer the ID from the canonical name; throw an error if the ID is already in use for another Person and no disambiguating information like suffix or ORCID is available. By default, after the ID is set, calls specify_id_on_items().)
• .specify_id_on_items() (Precondition: verified.)

[mbollmann]

I added .merge_into() onto #7570.

For .make_explicit() (I think we can keep the old name), I’m debating with myself whether we should have the suffix logic inside the library; as an extra argument, suffix potentially does nothing (if the ID can already be set to the inferred one, or if newid is set), and nothing else in the code currently cares about our ID naming convention. I’m thinking:

• .make_explicit() without arguments will try to generate the ID automatically, from canonical name and potentially ORCID.
• .make_explicit(new_id) just uses the supplied new_id instead.
• .make_explicit(new_id, skip_setting_ids=True) only changes the ID and is_explicit status, without setting IDs (phrased negatively as a skip_ argument to indicate that the previous behavior is still the default).

I understand this means that some of the logic which ID the person should get needs to be in the calling code, but maybe this would actually be clearer? For example

pid = person.canonical_name.slugify()
suffix = "abc"  # from some manually-set argument, presumably

try:
    person.make_explicit(pid)
except AnthologyException:
    person.make_explicit(f"{pid}-{suffix}")

[nschneid]

That logic isn't quite right because if we have a suffix and an ORCID, we want the suffix to take priority.

Maybe the ID logic should live in utils.ids? E.g. construct_author_id(name, suffix=None, orcid=None), raising an exception if not enough info is provided to create a unique ID? Then

pid = construct_author_id(person.canonical_name, "abc", person.orcid)
person.make_explicit(pid)

The Person method signatures could then be

• .make_explicit(pid=None, skip_setting_ids=False)
  • if pid is None, calls construct_author_id(person.canonical_name, orcid=person.orcid)
• .set_id_on_items(exclude_items=[])
  • Right now I am encountering cases where an author page has lots of papers and almost all of them belong to the author I am verifying, hence the optional parameter, which would be a list of Paper or Volume objects.

[mbollmann]

Maybe the ID logic should live in utils.ids? E.g. construct_author_id(name, suffix=None, orcid=None), raising an exception if not enough info is provided to create a unique ID?

That wouldn’t work as we also need to check the PersonIndex. After some deliberation, I decided to create this function on the PersonIndex instead, i.e. PersonIndex.generate_person_id(), in analogy to Volume.generate_paper_id(). A nice side-effect is that we can use exactly the same function during ingestion, encapsulating the ID generation logic there.