fix: Split Funding and Investigators (#430)

* fix: Split Funding and Investigators

* linter fixes

* interrogate

* fix: updates get_investigators tests

* tests: adds extra unit test

* integration test

---------

Co-authored-by: Jon Young <104453205+jtyoung84@users.noreply.github.com>

Author: mekhlakapoor

scripts/integration_test.py

@@ -36,6 +36,7 @@
         subject_id="804670",
         project_name="Learning mFISH-V1omFISH",
         modalities=[Modality.POPHYS, Modality.BEHAVIOR_VIDEOS, Modality.BEHAVIOR],
+        metadata_service_url="http://aind-metadata-service-dev", # DEV metadata service for testing
     )
 
     job = GatherMetadataJob(settings=settings)
@@ -72,7 +73,7 @@
         print("✗ Failed to fetch subject")
 
     print("\nFetching procedures from metadata service...")
-    procedures = job.get_procedures()
+    procedures = job.get_procedures(subject_id="804670")
     if procedures:
         print(f"✓ Procedures fetched for subject: {procedures.get('subject_id')}")
         if procedures.get("subject_procedures"):
@@ -83,14 +84,16 @@
         print("✗ Failed to fetch procedures")
 
     print("\nFetching funding from metadata service...")
-    funding_source, investigators = job.get_funding()
+    funding_source = job.get_funding()
     if funding_source:
         print(f"✓ Funding fetched: {len(funding_source)} funding source(s)")
         for fund in funding_source:
             print(f"  Funder: {fund.get('funder', {}).get('name', 'Unknown')}")
     else:
         print("✗ No funding information found")
 
+    print("\nFetching investigators from metadata service...")
+    investigators = job.get_investigators()
     if investigators:
         print(f"✓ Investigators fetched: {len(investigators)} investigator(s)")
         for inv in investigators:

src/aind_metadata_mapper/base.py

@@ -1,6 +1,7 @@
 """Base classes for metadata mappers"""
 
 from pathlib import Path
+
 from pydantic import BaseModel
 
 

src/aind_metadata_mapper/fip/mapper.py

@@ -303,9 +303,7 @@ def transform(
 
         # Get timing from all data streams (handle multiple epochs)
         # Find earliest start_time and latest end_time across all epochs
-        start_times = [
-            ensure_timezone(normalize_utc_timezone(ds["start_time"])) for ds in data_streams
-        ]
+        start_times = [ensure_timezone(normalize_utc_timezone(ds["start_time"])) for ds in data_streams]
         end_times = [ensure_timezone(normalize_utc_timezone(ds["end_time"])) for ds in data_streams]
 
         earliest_start = min(start_times)

src/aind_metadata_mapper/gather_metadata.py

@@ -133,46 +133,48 @@ def _get_prefixed_files_from_directory(self, directory: str, file_name_prefix: s
                 contents.append(json.load(f))
         return contents
 
-    def get_funding(self) -> tuple[list, list]:
-        """Get funding and investigators metadata from the V2 endpoint
+    def get_funding(self) -> list:
+        """Get funding metadata from the V2 endpoint
 
         Returns
         -------
-        tuple[list, list]
-            A tuple of (funding_source, investigators)
+        list
+            A list of funding sources
         """
         if not self.settings.project_name:
-            return [], []
+            return []
 
         funding_url = f"{self.settings.metadata_service_url}" f"/api/v2/funding/{self.settings.project_name}"
         funding_info = metadata_service_helper(funding_url)
+        return funding_info if funding_info else []
 
-        if not funding_info:
-            return [], []
-
-        investigators = []
-        parsed_funding_info = []
-
-        for f in funding_info:
-            project_investigators = f.get("investigators", [])
-            investigators.extend(project_investigators)
+    def get_investigators(self) -> list:
+        """Get investigators metadata from the V2 endpoint
 
-            funding_info_without_investigators = {k: v for k, v in f.items() if k != "investigators"}
-            parsed_funding_info.append(funding_info_without_investigators)
+        Returns
+        -------
+        list
+            A list of investigators
+        """
+        if not self.settings.project_name:
+            return []
 
+        investigators_url = (
+            f"{self.settings.metadata_service_url}" f"/api/v2/investigators/{self.settings.project_name}"
+        )
+        investigators_info = metadata_service_helper(investigators_url)
+        if investigators_info is None:
+            return []
         # Deduplicate investigators by name and sort
         seen_names = set()
         unique_investigators = []
-        for investigator in investigators:
+        for investigator in investigators_info:
             name = investigator.get("name", "")
             if name and name not in seen_names:
                 seen_names.add(name)
                 unique_investigators.append(investigator)
-
         unique_investigators.sort(key=lambda x: x.get("name", ""))
-        investigators_list = unique_investigators
-
-        return parsed_funding_info, investigators_list
+        return unique_investigators
 
     def build_data_description(self, acquisition_start_time: str, subject_id: str) -> dict:
         """Build data description metadata"""
@@ -184,14 +186,13 @@ def build_data_description(self, acquisition_start_time: str, subject_id: str) -
             logging.debug(f"Using existing {file_name}.")
             return self._get_file_from_user_defined_directory(file_name=file_name)
 
-        acquisition_start_time = normalize_utc_timezone(
-            acquisition_start_time
-        )  # remove when we're past Python 3.11
+        acquisition_start_time = normalize_utc_timezone(acquisition_start_time)  # remove when we're past Python 3.11
         creation_time = datetime.fromisoformat(acquisition_start_time)
         logging.info(f"Using acquisition start time: {creation_time}")
 
         # Get funding information
-        funding_source, investigators = self.get_funding()
+        funding_source = self.get_funding()
+        investigators = self.get_investigators()
 
         # Get modalities
         modalities = self.settings.modalities
@@ -513,9 +514,7 @@ def _validate_acquisition_start_time(self, acquisition_start_time: str) -> str:
         ValueError
             If acquisition_start_time doesn't match settings and raise_if_invalid is True
         """
-        acquisition_start_time = normalize_utc_timezone(
-            acquisition_start_time
-        )  # remove when we're past Python 3.11
+        acquisition_start_time = normalize_utc_timezone(acquisition_start_time)  # remove when we're past Python 3.11
         local_acq_start_time = datetime.fromisoformat(acquisition_start_time)
 
         if self.settings.acquisition_start_time and local_acq_start_time != self.settings.acquisition_start_time:

tests/resources/metadata_service/funding_response.json

@@ -1 +1 @@
-[{"object_type":"Funding information","funder":{"name":"Allen Institute","abbreviation":"AI","registry":"Research Organization Registry (ROR)","registry_identifier":"03cpe7c52"},"grant_number":null,"fundee":[{"object_type":"Person","name":"Marina Garrett","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null},{"object_type":"Person","name":"Peter Groblewski","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null},{"object_type":"Person","name":"Shawn Olsen","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null}],"investigators":[{"object_type":"Person","name":"Marina Garrett","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null},{"object_type":"Person","name":"Peter Groblewski","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null},{"object_type":"Person","name":"Shawn Olsen","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null}]},{"object_type":"Funding information","funder":{"name":"National Institute of Mental Health","abbreviation":"NIMH","registry":"Research Organization Registry (ROR)","registry_identifier":"04xeg9z08"},"grant_number":"U01MH130907","fundee":[{"object_type":"Person","name":"Marina Garrett","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null}],"investigators":[{"object_type":"Person","name":"Marina Garrett","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null},{"object_type":"Person","name":"Peter Groblewski","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null},{"object_type":"Person","name":"Shawn Olsen","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null}]}]
+[{"object_type":"Funding","funder":{"name":"Allen Institute","abbreviation":"AI","registry":"Research Organization Registry (ROR)","registry_identifier":"03cpe7c52"},"grant_number":null,"fundee":[{"object_type":"Person","name":"Marina Garrett","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null},{"object_type":"Person","name":"Peter Groblewski","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null},{"object_type":"Person","name":"Shawn Olsen","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null}]}]

tests/resources/metadata_service/investigators_response.json

@@ -0,0 +1 @@
+[{"object_type":"Person","name":"Marina Garrett","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null},{"object_type":"Person","name":"Peter Groblewski","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null},{"object_type":"Person","name":"Shawn Olsen","registry":"Open Researcher and Contributor ID (ORCID)","registry_identifier":null}]

tests/test_gather_metadata.py

@@ -12,7 +12,6 @@
 from pathlib import Path
 from unittest.mock import MagicMock, mock_open, patch
 
-from aind_data_schema.components.identifiers import Person
 from aind_data_schema.core.acquisition import Acquisition
 from aind_data_schema_models.modalities import Modality
 from aind_data_schema_models.organizations import Organization
@@ -290,11 +289,11 @@ def test_get_funding_no_project_name(self, mock_makedirs, mock_get):
                 acquisition_start_time=datetime(2023, 1, 1, 12, 0, 0),
             )
         )
-        funding, investigators = job_no_project.get_funding()
+        funding = job_no_project.get_funding()
 
         self.assertEqual(funding, [])
-        self.assertEqual(investigators, [])
         mock_get.assert_not_called()
+        mock_makedirs.assert_called()
 
     @patch("requests.get")
     def test_get_funding_success_single_result(self, mock_get):
@@ -305,20 +304,14 @@ def test_get_funding_success_single_result(self, mock_get):
             {
                 "funder": "Test Funder",
                 "award_number": "12345",
-                "investigators": [
-                    Person(name="John Doe").model_dump(),
-                    Person(name="Jane Smith").model_dump(),
-                ],
             }
         ]
         mock_get.return_value = mock_response
 
-        funding, investigators = self.job.get_funding()
+        funding = self.job.get_funding()
 
         expected_funding = [{"funder": "Test Funder", "award_number": "12345"}]
         self.assertEqual(funding, expected_funding)
-        self.assertEqual(len(investigators), 2)
-        self.assertIsInstance(investigators[0], dict)
 
     @patch("requests.get")
     def test_get_funding_success_multiple_results(self, mock_get):
@@ -329,58 +322,109 @@ def test_get_funding_success_multiple_results(self, mock_get):
             {
                 "funder": "Funder 1",
                 "award_number": "111",
-                "investigators": [{"name": "Alice"}],
             },
             {
                 "funder": "Funder 2",
                 "award_number": "222",
-                "investigators": [{"name": "Bob"}, {"name": "Charlie"}],
             },
         ]
         mock_get.return_value = mock_response
 
-        funding, investigators = self.job.get_funding()
+        funding = self.job.get_funding()
 
         self.assertEqual(len(funding), 2)
-        self.assertEqual(len(investigators), 3)
 
     @patch("requests.get")
-    def test_get_funding_empty_investigators(self, mock_get):
-        """Test get_funding with empty investigators"""
+    @patch("os.makedirs")
+    def test_get_investigators_no_project_name(self, mock_makedirs, mock_get):
+        """Test get_investigators when no project name is provided"""
+        job_no_project = GatherMetadataJob(
+            JobSettings(
+                metadata_dir="/test",
+                output_dir="/test/output",
+                subject_id="test_subject",
+                project_name="",
+                modalities=[Modality.ECEPHYS],
+                acquisition_start_time=datetime(2023, 1, 1, 12, 0, 0),
+            )
+        )
+        investigators = job_no_project.get_investigators()
+
+        self.assertEqual(investigators, [])
+        mock_get.assert_not_called()
+        mock_makedirs.assert_called()
+
+    @patch("aind_metadata_mapper.gather_metadata.metadata_service_helper")
+    @patch("os.makedirs")
+    def test_get_investigators_helper_issue(self, mock_makedirs, mock_metadata_helper):
+        """Test get_investigators when helper function returns None"""
+        job_no_project = GatherMetadataJob(
+            JobSettings(
+                metadata_dir="/test",
+                output_dir="/test/output",
+                subject_id="test_subject",
+                project_name="Some Project",
+                modalities=[Modality.ECEPHYS],
+                acquisition_start_time=datetime(2023, 1, 1, 12, 0, 0),
+            )
+        )
+        mock_metadata_helper.return_value = None
+        investigators = job_no_project.get_investigators()
+
+        self.assertEqual(investigators, [])
+        mock_makedirs.assert_called()
+
+    @patch("requests.get")
+    def test_get_investigators_success_single_result(self, mock_get):
+        """Test get_investigators with successful single result"""
         mock_response = MagicMock()
         mock_response.status_code = 200
         mock_response.json.return_value = [
             {
-                "funder": "Test Funder",
-                "award_number": "12345",
-                "investigators": [],
+                "object_type": "Person",
+                "name": "Jane Doe",
+                "registry": "Open Researcher and Contributor ID (ORCID)",
+                "registry_identifier": None,
             }
         ]
         mock_get.return_value = mock_response
 
-        funding, investigators = self.job.get_funding()
-
-        self.assertEqual(len(investigators), 0)
+        investigators = self.job.get_investigators()
 
-    @patch("aind_metadata_mapper.utils.metadata_service_helper")
-    def test_get_funding_http_error(self, mock_helper):
-        """Test get_funding with HTTP error"""
-        mock_helper.return_value = None
-
-        funding, investigators = self.job.get_funding()
-
-        self.assertEqual(funding, [])
-        self.assertEqual(investigators, [])
+        expected_investigators = [
+            {
+                "object_type": "Person",
+                "name": "Jane Doe",
+                "registry": "Open Researcher and Contributor ID (ORCID)",
+                "registry_identifier": None,
+            }
+        ]
+        self.assertEqual(investigators, expected_investigators)
 
-    @patch("aind_metadata_mapper.utils.metadata_service_helper")
-    def test_get_funding_request_exception(self, mock_helper):
-        """Test get_funding with request exception"""
-        mock_helper.return_value = None
+    @patch("requests.get")
+    def test_get_investigators_success_multiple_results(self, mock_get):
+        """Test get_investigators with successful multiple results"""
+        mock_response = MagicMock()
+        mock_response.status_code = 200
+        mock_response.json.return_value = [
+            {
+                "object_type": "Person",
+                "name": "Jane Doe",
+                "registry": "Open Researcher and Contributor ID (ORCID)",
+                "registry_identifier": None,
+            },
+            {
+                "object_type": "Person",
+                "name": "John Smith",
+                "registry": "Open Researcher and Contributor ID (ORCID)",
+                "registry_identifier": None,
+            },
+        ]
+        mock_get.return_value = mock_response
 
-        funding, investigators = self.job.get_funding()
+        investigators = self.job.get_investigators()
 
-        self.assertEqual(funding, [])
-        self.assertEqual(investigators, [])
+        self.assertEqual(len(investigators), 2)
 
     # Tests for _write_json_file method
     @patch("builtins.open", new_callable=mock_open)
@@ -416,14 +460,31 @@ def test_build_data_description_existing_file(self, mock_get_file, mock_file_exi
 
     @patch.object(GatherMetadataJob, "_does_file_exist_in_user_defined_dir")
     @patch.object(GatherMetadataJob, "get_funding")
+    @patch.object(GatherMetadataJob, "get_investigators")
     @patch("aind_metadata_mapper.gather_metadata.datetime")
-    def test_build_data_description_new_file(self, mock_datetime, mock_get_funding, mock_file_exists):
+    def test_build_data_description_new_file(
+        self, mock_datetime, mock_get_investigators, mock_get_funding, mock_file_exists
+    ):
         """Test build_data_description when creating new file"""
         mock_file_exists.return_value = False
-        mock_get_funding.return_value = (
-            [{"funder": Organization.NIMH, "grant_number": "12345"}],
-            [{"name": "Test Investigator"}],
-        )
+        mock_get_funding.return_value = [
+            {
+                "object_type": "Funding",
+                "funder": Organization.NIMH,
+                "grant_number": "12345",
+                "fundee": [
+                    {
+                        "object_type": "Person",
+                        "name": "Jane Doe",
+                        "registry": "Addgene (ADDGENE)",
+                        "registry_identifier": None,
+                    }
+                ],
+            }
+        ]
+        mock_get_investigators.return_value = [
+            {"object_type": "Person", "name": "Jane Doe", "registry": "Addgene (ADDGENE)", "registry_identifier": None}
+        ]
         mock_datetime.now.return_value = datetime(2023, 1, 1, 12, 0, 0)
 
         result = self.job.build_data_description(acquisition_start_time="2023-01-01T12:00:00", subject_id="123456")