Timx 291 orchestration

tests/sources/test_transformer.py

@@ -28,59 +28,49 @@ def test_transformer_get_transformer_source_wrong_module_path_raises_error():
         Transformer.get_transformer("bad-module-path")
 
 
-def test_create_dates_and_locations_from_publishers_success(
-    timdex_record_required_fields,
-):
-    assert timdex_record_required_fields.dates is None
-    assert timdex_record_required_fields.locations is None
+def test_create_dates_from_publishers_success(timdex_record_required_fields):
     timdex_record_required_fields.publishers = [
         timdex.Publisher(name="Publisher", date="2018", location="Location")
     ]
-    timdex_record = Transformer.create_dates_and_locations_from_publishers(
-        timdex_record_required_fields
-    )
-    assert timdex_record.dates == [timdex.Date(kind="Publication date", value="2018")]
-    assert timdex_record.locations == [
-        timdex.Location(value="Location", kind="Place of Publication")
-    ]
+    assert list(
+        Transformer.create_dates_from_publishers(timdex_record_required_fields)
+    ) == [timdex.Date(kind="Publication date", value="2018")]
 
 
-def test_create_dates_and_locations_from_publishers_drops_unparseable_dates(
+def test_create_dates_from_publishers_drops_unparseable_dates(
     caplog, timdex_record_required_fields
 ):
     caplog.set_level("DEBUG")
-    assert timdex_record_required_fields.dates is None
-    assert timdex_record_required_fields.locations is None
     timdex_record_required_fields.publishers = [
         timdex.Publisher(name="Publisher", date="Date", location="Location")
     ]
-    timdex_record = Transformer.create_dates_and_locations_from_publishers(
-        timdex_record_required_fields
+    assert (
+        list(Transformer.create_dates_from_publishers(timdex_record_required_fields))
+        == []
     )
-    assert timdex_record.dates is None
-    assert timdex_record.locations == [
-        timdex.Location(value="Location", kind="Place of Publication")
-    ]
     assert (
         "Record ID 'cool-repo:123' has a date that couldn't be parsed: 'Date'"
         in caplog.text
     )
 
 
+def test_create_locations_from_publishers_success(timdex_record_required_fields):
+    timdex_record_required_fields.publishers = [
+        timdex.Publisher(name="Publisher", date="2018", location="Location")
+    ]
+    assert list(
+        Transformer.create_locations_from_publishers(timdex_record_required_fields)
+    ) == [timdex.Location(value="Location", kind="Place of Publication")]
+
+
 def test_create_locations_from_spatial_subjects_success(timdex_record_required_fields):
     timdex_record_required_fields.subjects = [
         timdex.Subject(value=["Some city, Some country"], kind="Dublin Core; Spatial"),
         timdex.Subject(value=["City 1", "City 2"], kind="Dublin Core; Spatial"),
     ]
-    assert timdex_record_required_fields.locations is None
-    timdex_record = Transformer.create_locations_from_spatial_subjects(
-        timdex_record_required_fields
-    )
-    assert timdex_record.subjects == [
-        timdex.Subject(value=["Some city, Some country"], kind="Dublin Core; Spatial"),
-        timdex.Subject(value=["City 1", "City 2"], kind="Dublin Core; Spatial"),
-    ]
-    assert timdex_record.locations == [
+    assert list(
+        Transformer.create_locations_from_spatial_subjects(timdex_record_required_fields)
+    ) == [
         timdex.Location(value="Some city, Some country", kind="Place Name"),
         timdex.Location(value="City 1", kind="Place Name"),
         timdex.Location(value="City 2", kind="Place Name"),

tests/sources/xml/test_marc.py

@@ -298,6 +298,8 @@ def test_marc_record_all_fields_transform_correctly():
             ),
             timdex.Location(value="Austria - Vienna", kind="Hierarchical Place Name"),
             timdex.Location(value="New York", kind="Place of Publication"),
+            timdex.Location(value="New York", kind="Place of Publication"),
+            timdex.Location(value="France", kind="Place of Publication"),
         ],
         notes=[
             timdex.Note(
@@ -675,6 +677,7 @@ def test_marc_record_attribute_and_subfield_variations_transforms_correctly():
                 kind="Hierarchical Place Name",
             ),
             timdex.Location(value="a", kind="Place of Publication"),
+            timdex.Location(value="a", kind="Place of Publication"),
         ],
         notes=[
             timdex.Note(value=["c"], kind="Title Statement of Responsibility"),

transmogrifier/sources/transformer.py

@@ -351,8 +351,21 @@ def generate_derived_fields(
 
         May not be overridden.
         """
-        self.create_dates_and_locations_from_publishers(timdex_record)
-        self.create_locations_from_spatial_subjects(timdex_record)
+        derived_dates = list(self.create_dates_from_publishers(timdex_record))
+        if not timdex_record.dates:
+            timdex_record.dates = derived_dates if derived_dates else None
+        else:
+            timdex_record.dates.extend(derived_dates)
+
+        derived_locations: list[timdex.Location] = []
+        derived_locations.extend(self.create_locations_from_publishers(timdex_record))
+        derived_locations.extend(
+            self.create_locations_from_spatial_subjects(timdex_record)
+        )
+        if not timdex_record.locations:
+            timdex_record.locations = derived_locations if derived_locations else None
+        else:
+            timdex_record.locations.extend(derived_locations)
 
         if timdex_record.citation is None:
             timdex_record.citation = generate_citation(timdex_record)
@@ -363,63 +376,55 @@ def generate_derived_fields(
 
     @final
     @staticmethod
-    def create_dates_and_locations_from_publishers(
+    def create_dates_from_publishers(
         timdex_record: timdex.TimdexRecord,
-    ) -> timdex.TimdexRecord:
-        """Add Date and Location objects based on data in publishers field.
+    ) -> Iterator[timdex.Date]:
+        """Derive Date objects based on data in publishers field.
 
         Args:
             timdex_record: A TimdexRecord class instance.
         """
-        if not timdex_record.publishers:
-            return timdex_record
+        if timdex_record.publishers:
+            for publisher in timdex_record.publishers:
+                if publisher.date and validate_date(
+                    publisher.date, timdex_record.timdex_record_id
+                ):
+                    yield timdex.Date(kind="Publication date", value=publisher.date)
 
-        for publisher in timdex_record.publishers:
-            if publisher.date and validate_date(
-                publisher.date, timdex_record.timdex_record_id
-            ):
-                publisher_date = timdex.Date(
-                    kind="Publication date", value=publisher.date
-                )
-                if not timdex_record.dates:
-                    timdex_record.dates = [publisher_date]
-                elif publisher_date not in timdex_record.dates:
-                    timdex_record.dates.append(publisher_date)
-
-            if publisher.location:
-                publisher_location = timdex.Location(
-                    kind="Place of Publication", value=publisher.location
-                )
-                if not timdex_record.locations:
-                    timdex_record.locations = [publisher_location]
-                elif publisher_location not in timdex_record.locations:
-                    timdex_record.locations.append(publisher_location)
-        return timdex_record
+    @final
+    @staticmethod
+    def create_locations_from_publishers(
+        timdex_record: timdex.TimdexRecord,
+    ) -> Iterator[timdex.Location]:
+        """Derive Location objects based on data in publishers field.
+
+        Args:
+            timdex_record: A TimdexRecord class instance.
+        """
+        if timdex_record.publishers:
+            for publisher in timdex_record.publishers:
+                if publisher.location:
+                    yield timdex.Location(
+                        kind="Place of Publication", value=publisher.location
+                    )
 
     @final
     @staticmethod
     def create_locations_from_spatial_subjects(
         timdex_record: timdex.TimdexRecord,
-    ) -> timdex.TimdexRecord:
-        """Add Location objects for spatial subjects.
+    ) -> Iterator[timdex.Location]:
+        """Derive Location objects from a TimdexRecord's spatial subjects.
 
         Args:
            timdex_record: A TimdexRecord class instance.
         """
-        if not timdex_record.subjects:
-            return timdex_record
-
-        spatial_subjects = [
-            subject
-            for subject in timdex_record.subjects
-            if subject.kind == "Dublin Core; Spatial" and subject.value is not None
-        ]
-
-        for subject in spatial_subjects:
-            for place_name in subject.value:
-                subject_location = timdex.Location(value=place_name, kind="Place Name")
-                if not timdex_record.locations:
-                    timdex_record.locations = [subject_location]
-                elif subject_location not in timdex_record.locations:
-                    timdex_record.locations.append(subject_location)
-        return timdex_record
+        if timdex_record.subjects:
+            spatial_subjects = [
+                subject
+                for subject in timdex_record.subjects
+                if subject.kind == "Dublin Core; Spatial" and subject.value is not None
+            ]
+
+            for subject in spatial_subjects:
+                for place_name in subject.value:
+                    yield timdex.Location(value=place_name, kind="Place Name")