Repository: sciunto-org/python-bibtexparser
Pull request #422 (2023-11-23 00:06:47)

Add merging of names in last-name-first style
I added a flag to `MergeNameParts` so the user can decide whether to use first-name-first or last-name-first.
I made last-name-first the default.

I slightly modified one existing test group to support the flag.

I added a new test group `test_merge_last_name_first_inverse` which ensures that merging with commas is the inverse of splitting. I skip this test for cases including a `/`, since this property doesn't hold for some of those cases.

I also had to do a bit of strange stuff with the typing imports. The flag I added is a union of literal strings, but literal string types aren't available in Python 3.7. So I try importing a literal type and if that doesn't work then I just say the type of the flag is "str", which is too general but the closest I can get 

mlutze: > Thanks for your work on this. Just to avoid a misunderstanding: I wont review this PR while it is marked as draft.

Understood. There are still some changes to testing and documentation I'd like to make before submitting for review.
mlutze: @MiWeiss This is ready for review.

Patch:
diff --git a/bibtexparser/middlewares/enclosing.py b/bibtexparser/middlewares/enclosing.py
index f4a72b2..8aa0c32 100644
--- a/bibtexparser/middlewares/enclosing.py
+++ b/bibtexparser/middlewares/enclosing.py
@@ -37,8 +37,8 @@ class RemoveEnclosingMiddleware(BlockMiddleware):
         )
 
     # docstr-coverage: inherited
-    @staticmethod
-    def metadata_key() -> str:
+    @classmethod
+    def metadata_key(cls) -> str:
         return REMOVED_ENCLOSING_KEY
 
     @staticmethod
@@ -109,8 +109,8 @@ class AddEnclosingMiddleware(BlockMiddleware):
         self._enclose_integers = enclose_integers
 
     # docstr-coverage: inherited
-    @staticmethod
-    def metadata_key() -> str:
+    @classmethod
+    def metadata_key(cls) -> str:
         return "remove_enclosing"
 
     def _enclose(
diff --git a/bibtexparser/middlewares/interpolate.py b/bibtexparser/middlewares/interpolate.py
index 62e9b70..d9e1076 100644
--- a/bibtexparser/middlewares/interpolate.py
+++ b/bibtexparser/middlewares/interpolate.py
@@ -28,8 +28,8 @@ class ResolveStringReferencesMiddleware(LibraryMiddleware):
         super().__init__(allow_inplace_modification)
 
     # docstr-coverage: inherited
-    @staticmethod
-    def metadata_key() -> str:
+    @classmethod
+    def metadata_key(cls) -> str:
         return "ResolveStringReferences"
 
     # docstr-coverage: inherited
diff --git a/bibtexparser/middlewares/middleware.py b/bibtexparser/middlewares/middleware.py
index 2329eb9..1f0f2c5 100644
--- a/bibtexparser/middlewares/middleware.py
+++ b/bibtexparser/middlewares/middleware.py
@@ -62,14 +62,12 @@ class BlockMiddleware(Middleware, abc.ABC):
     except if `allow_inplace_modification` is true.
     """
 
-    @staticmethod
-    @abc.abstractmethod
-    def metadata_key() -> str:
+    @classmethod
+    def metadata_key(cls) -> str:
         """Identifier of the middleware.
-
         This key is used to identify the middleware in a blocks metadata.
         """
-        pass
+        return cls.__name__
 
     # docstr-coverage: inherited
     def transform(self, library: "Library") -> "Library":
diff --git a/bibtexparser/middlewares/month.py b/bibtexparser/middlewares/month.py
index 16fba7b..0dbd621 100644
--- a/bibtexparser/middlewares/month.py
+++ b/bibtexparser/middlewares/month.py
@@ -80,8 +80,8 @@ class MonthLongStringMiddleware(_MonthInterpolator):
     and unenclosed."""
 
     # docstr-coverage: inherited
-    @staticmethod
-    def metadata_key() -> str:
+    @classmethod
+    def metadata_key(cls) -> str:
         return "MonthLongStringMiddleware"
 
     # docstr-coverage: inherited
@@ -124,8 +124,8 @@ class MonthAbbreviationMiddleware(_MonthInterpolator):
     The created abbreviations are always lowercase and unenclosed."""
 
     # docstr-coverage: inherited
-    @staticmethod
-    def metadata_key() -> str:
+    @classmethod
+    def metadata_key(cls) -> str:
         return "MonthAbbreviationMiddleware"
 
     # docstr-coverage: inherited
@@ -159,8 +159,8 @@ class MonthIntMiddleware(_MonthInterpolator):
     The created int-months are always integers and unenclosed."""
 
     # docstr-coverage: inherited
-    @staticmethod
-    def metadata_key() -> str:
+    @classmethod
+    def metadata_key(cls) -> str:
         return "MonthIntMiddleware"
 
     # docstr-coverage: inherited
diff --git a/bibtexparser/middlewares/names.py b/bibtexparser/middlewares/names.py
index 9b6d308..11d4d42 100644
--- a/bibtexparser/middlewares/names.py
+++ b/bibtexparser/middlewares/names.py
@@ -5,7 +5,7 @@ Much of the code is taken from Blair Bonnetts never merged v0 pull request
 """
 import abc
 import dataclasses
-from typing import List, Tuple
+from typing import List, Literal, Tuple
 
 from bibtexparser.model import Block, Entry, Field, MiddlewareErrorBlock
 
@@ -62,8 +62,8 @@ class SeparateCoAuthors(_NameTransformerMiddleware):
     """Middleware to separate multi-person fields (e.g. co-authors, co-editors)."""
 
     # docstr-coverage: inherited
-    @staticmethod
-    def metadata_key() -> str:
+    @classmethod
+    def metadata_key(cls) -> str:
         return "separate_coauthors"
 
     # docstr-coverage: inherited
@@ -75,8 +75,8 @@ class MergeCoAuthors(_NameTransformerMiddleware):
     """Middleware to merge multi-person-list fields (e.g. co-authors, co-editors)."""
 
     # docstr-coverage: inherited
-    @staticmethod
-    def metadata_key() -> str:
+    @classmethod
+    def metadata_key(cls) -> str:
         return "merge_coauthors"
 
     # docstr-coverage: inherited
@@ -114,6 +114,35 @@ class NameParts:
             ]
         )
 
+    @property
+    def merge_last_name_first(self) -> str:
+        """Merging the name parts into a single string, last-name-first (with commas) format.
+
+        The structure of the output is: `von Last, Jr, First`
+        """
+
+        def escape_last_slash(string: str) -> str:
+            """Escape the last slash in a string if it is not escaped."""
+            # Find the number of trailing slashes
+            stripped = string.rstrip("\\")
+            num_slashes = len(string) - len(stripped)
+            if num_slashes % 2 == 0:
+                # Even number: everything is escaped
+                return string
+            else:
+                # Odd number: need to escape one.
+                return string + "\\"
+
+        first = " ".join(self.first) if self.first else None
+        von = " ".join(self.von) if self.von else None
+        last = " ".join(self.last) if self.last else None
+        jr = " ".join(self.jr) if self.jr else None
+
+        von_last = " ".join(name for name in [von, last] if name)
+        return ", ".join(
+            escape_last_slash(name) for name in [von_last, jr, first] if name
+        )
+
 
 class SplitNameParts(_NameTransformerMiddleware):
     """Middleware to split a persons name into its parts (first, von, last, jr).
@@ -123,8 +152,8 @@ class SplitNameParts(_NameTransformerMiddleware):
     """
 
     # docstr-coverage: inherited
-    @staticmethod
-    def metadata_key() -> str:
+    @classmethod
+    def metadata_key(cls) -> str:
         return "split_name_parts"
 
     def _transform_field_value(self, name) -> List[NameParts]:
@@ -142,18 +171,38 @@ class MergeNameParts(_NameTransformerMiddleware):
     """Middleware to merge a persons name parts (first, von, last, jr) into a single string.
 
     Name fields (e.g. author, editor, translator) are expected to be lists of NameParts.
+
+    The merging style indicates whether:
+    - the merging is done without commas in first-name-first order ("first"), or
+    - the merging is done with commas in last-name-first order ("last").
     """
 
+    def __init__(
+        self,
+        style: Literal["last", "first"] = "last",
+        allow_inplace_modification: bool = True,
+        name_fields: Tuple[str] = ("author", "editor", "translator"),
+    ):
+        self.style = style
+        super().__init__(allow_inplace_modification, name_fields)
+
     # docstr-coverage: inherited
-    @staticmethod
-    def metadata_key() -> str:
+    @classmethod
+    def metadata_key(cls) -> str:
         return "merge_name_parts"
 
     def _transform_field_value(self, name) -> List[str]:
         if not isinstance(name, list) and all(isinstance(n, NameParts) for n in name):
             raise ValueError("Expected a list of NameParts, got {}. ".format(name))
 
-        return [n.merge_first_name_first for n in name]
+        if self.style == "last":
+            return [n.merge_last_name_first for n in name]
+        elif self.style == "first":
+            return [n.merge_first_name_first for n in name]
+        else:
+            raise ValueError(
+                """Expected "first" or "last" style, got {}. """.format(self.style)
+            )
 
 
 def parse_single_name_into_parts(name, strict=True):
@@ -229,33 +278,38 @@ def parse_single_name_into_parts(name, strict=True):
     for char in nameiter:
         # An escape.
         if char == "\\":
-            escaped = next(nameiter)
-
-            # BibTeX doesn't allow whitespace escaping. Copy the slash and fall
-            # through to the normal case to handle the whitespace.
-            if escaped in whitespace:
-                word.append(char)
-                char = escaped
-
-            else:
-                # Is this the first character in a brace?
-                if bracestart:
-                    bracestart = False
-                    controlseq = escaped.isalpha()
-                    specialchar = True
-
-                # Can we use it to determine the case?
-                elif (case == -1) and escaped.isalpha():
-                    if escaped.isupper():
-                        case = 1
-                    else:
-                        case = 0
-
-                # Copy the escape to the current word and go to the next
-                # character in the input.
+            try:
+                escaped = next(nameiter)
+
+                # BibTeX doesn't allow whitespace escaping. Copy the slash and fall
+                # through to the normal case to handle the whitespace.
+                if escaped in whitespace:
+                    word.append(char)
+                    char = escaped
+
+                else:
+                    # Is this the first character in a brace?
+                    if bracestart:
+                        bracestart = False
+                        controlseq = escaped.isalpha()
+                        specialchar = True
+
+                    # Can we use it to determine the case?
+                    elif (case == -1) and escaped.isalpha():
+                        if escaped.isupper():
+                            case = 1
+                        else:
+                            case = 0
+
+                    # Copy the escape to the current word and go to the next
+                    # character in the input.
+                    word.append(char)
+                    word.append(escaped)
+                    continue
+
+            # If we're at the end of the string, then the \ is just a \.
+            except StopIteration:
                 word.append(char)
-                word.append(escaped)
-                continue
 
         # Start of a braced expression.
         if char == "{":
@@ -510,7 +564,11 @@ def split_multiple_persons_names(names):
 
         # Escaped character.
         if char == "\\":
-            next(namesiter)
+            try:
+                next(namesiter)
+            # If we're at the end of the string, then the \ is just a \.
+            except StopIteration:
+                pass
             pos += 1
             continue
 
diff --git a/bibtexparser/middlewares/sorting_entry_fields.py b/bibtexparser/middlewares/sorting_entry_fields.py
index 3995ee5..600924a 100644
--- a/bibtexparser/middlewares/sorting_entry_fields.py
+++ b/bibtexparser/middlewares/sorting_entry_fields.py
@@ -22,8 +22,8 @@ class SortFieldsAlphabeticallyMiddleware(BlockMiddleware):
         return entry
 
     # docstr-coverage: inherited
-    @staticmethod
-    def metadata_key() -> str:
+    @classmethod
+    def metadata_key(cls) -> str:
         return "sorted_fields_alphabetically"
 
 
@@ -71,6 +71,6 @@ class SortFieldsCustomMiddleware(BlockMiddleware):
         return entry
 
     # docstr-coverage: inherited
-    @staticmethod
-    def metadata_key() -> str:
+    @classmethod
+    def metadata_key(cls) -> str:
         return "sorted_fields_custom"
diff --git a/bibtexparser/model.py b/bibtexparser/model.py
index d3b99be..b53876a 100644
--- a/bibtexparser/model.py
+++ b/bibtexparser/model.py
@@ -318,6 +318,14 @@ class Entry(Block):
             return self.key
         return self.fields_dict[key].value
 
+    def __setitem__(self, key: str, value: Any):
+        """Dict-mimicking index.
+
+        This serves for partial v1.x backwards compatibility,
+        as well as for a shorthand for `set_field`.
+        """
+        self.set_field(Field(key, value))
+
     def items(self):
         """Dict-mimicking, for partial v1.x backwards compatibility.
 


Test patch:
diff --git a/tests/middleware_tests/test_block_middleware.py b/tests/middleware_tests/test_block_middleware.py
index eef9cce..9cfa3ed 100644
--- a/tests/middleware_tests/test_block_middleware.py
+++ b/tests/middleware_tests/test_block_middleware.py
@@ -30,9 +30,6 @@ class ConstantBlockMiddleware(BlockMiddleware):
     def transform_block(self, block, library):
         return self._const
 
-    def metadata_key():
-        return "ConstantBlockMiddleware"
-
 
 class LambdaBlockMiddleware(BlockMiddleware):
     """A middleware that applies a lambda to the input block"""
@@ -44,9 +41,6 @@ class LambdaBlockMiddleware(BlockMiddleware):
     def transform_block(self, block, library):
         return self._f(block)
 
-    def metadata_key():
-        return "LambdaBlockMiddleware"
-
 
 @pytest.mark.parametrize(
     ("middleware", "expected"),
diff --git a/tests/middleware_tests/test_names.py b/tests/middleware_tests/test_names.py
index fb5e6bc..8dde269 100644
--- a/tests/middleware_tests/test_names.py
+++ b/tests/middleware_tests/test_names.py
@@ -860,6 +860,39 @@ def test_split_name_into_parts(name, expected_as_dict, strict):
     assert result == expected
 
 
+@pytest.mark.parametrize(
+    "name, expected_as_dict", REGULAR_NAME_PARTS_PARSING_TEST_CASES
+)
+@pytest.mark.parametrize("strict", [True, False], ids=["strict", "non-strict"])
+def test_merge_last_name_first_inverse(name, expected_as_dict, strict):
+    """Tests that merging name parts using the last-name-first method
+    maintains the "semantics" of the name.
+
+    This property does not hold for certain values that contain '\\'.
+    """
+
+    # cases where either the last name or "von" part ends with an odd number of `\` cannot be handled,
+    # since in those cases the `,` is escaped when the name parts are put back together
+    def ends_with_odd_slash(names: List[str]) -> bool:
+        if len(names) == 0:
+            return False
+        name = names[-1]
+        count = 0
+        i = len(name) - 1
+        while i >= 0 and name[i] == "\\":
+            count += 1
+            i -= 1
+        return count % 2 == 1
+
+    if any(ends_with_odd_slash(name) for name in expected_as_dict.values()):
+        pytest.skip("Inverse property does not hold for names ending with '\\'")
+
+    nameparts = _dict_to_nameparts(expected_as_dict)
+    merged = nameparts.merge_last_name_first
+    resplit = parse_single_name_into_parts(merged, strict=strict)
+    assert resplit == nameparts
+
+
 @pytest.mark.parametrize("inplace", [True, False], ids=["inplace", "copy"])
 def test_separate_co_names_middleware(inplace):
     """Test coauthor, co-editor, splitting middleware.
@@ -966,7 +999,25 @@ def test_split_name_parts(inplace: bool):
 
 
 @pytest.mark.parametrize("inplace", [True, False], ids=["inplace", "copy"])
-def test_merge_name_parts(inplace: bool):
+@pytest.mark.parametrize(
+    ("style", "names"),
+    [
+        (
+            "first",
+            ["Amy Author", "Ben Bystander", "Carl Carpooler\\", "Donald Doctor\\\\"],
+        ),
+        (
+            "last",
+            [
+                "Author, Amy",
+                "Bystander, Ben",
+                "Carpooler\\\\, Carl",
+                "Doctor\\\\, Donald",
+            ],
+        ),
+    ],
+)
+def test_merge_name_parts(inplace: bool, style: str, names: List[str]):
     input_entry = Entry(
         start_line=0,
         raw="irrelevant-for-this-test",
@@ -980,13 +1031,15 @@ def test_merge_name_parts(inplace: bool):
                 value=[
                     NameParts(first=["Amy"], last=["Author"], von=[], jr=[]),
                     NameParts(first=["Ben"], last=["Bystander"], von=[], jr=[]),
+                    NameParts(first=["Carl"], last=["Carpooler\\"], von=[], jr=[]),
+                    NameParts(first=["Donald"], last=["Doctor\\\\"], von=[], jr=[]),
                 ],
             ),
         ],
     )
     original_copy = deepcopy(input_entry)
 
-    middleware = MergeNameParts(allow_inplace_modification=inplace)
+    middleware = MergeNameParts(style=style, allow_inplace_modification=inplace)
     transformed_library = middleware.transform(Library([input_entry]))
 
     assert len(transformed_library.entries) == 1
@@ -994,10 +1047,7 @@ def test_merge_name_parts(inplace: bool):
 
     transformed_entry = transformed_library.entries[0]
     assert transformed_entry.fields_dict["title"] == original_copy.fields_dict["title"]
-    assert transformed_entry.fields_dict["author"].value == [
-        "Amy Author",
-        "Ben Bystander",
-    ]
+    assert transformed_entry.fields_dict["author"].value == names
 
     # Make sure other attributes are not changed
     assert_nonfield_entry_attributes_unchanged(original_copy, transformed_entry)
diff --git a/tests/test_model.py b/tests/test_model.py
index a52894e..a2d5c3b 100644
--- a/tests/test_model.py
+++ b/tests/test_model.py
@@ -290,3 +290,24 @@ def test_entry_str():
     )
 
     assert str(entry) == expected
+
+
+def test_entry_fields_shorthand():
+    entry = Entry(
+        entry_type="article",
+        key="myEntry",
+        fields=[
+            Field("myFirstField", "firstValue"),
+            Field("mySecondField", "secondValue"),
+        ],
+    )
+
+    entry["myFirstField"] = "changed_value"
+    assert entry["myFirstField"] == "changed_value"
+    assert entry.fields_dict["myFirstField"].value == "changed_value"
+
+    entry["myNewField"] = "new_value"
+    assert entry["myNewField"] == "new_value"
+    assert entry.fields_dict["myNewField"].key == "myNewField"
+    assert entry.fields_dict["myNewField"].value == "new_value"
+    assert entry.fields_dict["myNewField"].start_line is None