Repository: catalystneuro/neuroconv

Issue:
[Bug]: AssertionError triggered when trying to add both OnePhotonSeries and TwoPhotonSeries
### What happened?

`AssertionError: Received metadata for 'OnePhotonSeries' but photon_series_type was not explicitly specified.`

Here it is assumed that either OnePhotonSeries or TwoPhotonSeries can co-exist in the same nwbfile. That I not necessarily true, e.g. dual acquisition of both two-photon and one-photon imaging ([paper](https://www.nature.com/articles/s41593-023-01498-y))
https://github.com/catalystneuro/neuroconv/blob/5a65e7417d90411bca3dce8312f8d6d483d30b05/src/neuroconv/tools/roiextractors/roiextractors.py#L402-L405

### Operating System

Linux

### Python Executable

Conda

### Python Version

3.9

### Package Versions

_No response_

### Code of Conduct

- [X] I agree to follow this project's [Code of Conduct](https://github.com/catalystneuro/neuroconv/blob/main/.github/CODE_OF_CONDUCT.rst)
- [X] Have you ensured this bug was not already [reported](https://github.com/catalystneuro/neuroconv/issues)?

Discussion:
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 90.95%. Comparing base [(`5a65e74`)](https://app.codecov.io/gh/catalystneuro/neuroconv/commit/5a65e7417d90411bca3dce8312f8d6d483d30b05?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) to head [(`51b7f90`)](https://app.codecov.io/gh/catalystneuro/neuroconv/commit/51b7f90b11fe70fee52984bde7a77090f34a668f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro).

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907/graphs/tree.svg?width=650&height=150&src=pr&token=h63OQoWehP&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro)](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro)

```diff
@@           Coverage Diff           @@
##             main     #907   +/-   ##
=======================================
  Coverage   90.95%   90.95%           
=======================================
  Files         125      125           
  Lines        7062     7062           
=======================================
  Hits         6423     6423           
  Misses        639      639           
```

| [Flag](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) | `90.95% <100.00%> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Files](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) | Coverage Δ | |
|---|---|---|
| [src/neuroconv/tools/roiextractors/roiextractors.py](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907?src=pr&el=tree&filepath=src%2Fneuroconv%2Ftools%2Froiextractors%2Froiextractors.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro#diff-c3JjL25ldXJvY29udi90b29scy9yb2lleHRyYWN0b3JzL3JvaWV4dHJhY3RvcnMucHk=) | `88.02% <100.00%> (ø)` | |

</details>
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 90.95%. Comparing base [(`5a65e74`)](https://app.codecov.io/gh/catalystneuro/neuroconv/commit/5a65e7417d90411bca3dce8312f8d6d483d30b05?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) to head [(`51b7f90`)](https://app.codecov.io/gh/catalystneuro/neuroconv/commit/51b7f90b11fe70fee52984bde7a77090f34a668f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro).
> Report is 5 commits behind head on main.

> :exclamation: **Current head 51b7f90 differs from pull request most recent head 504493a**
> 
> Please [upload](https://docs.codecov.com/docs/codecov-uploader) reports for the commit 504493a to get more accurate results.

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907/graphs/tree.svg?width=650&height=150&src=pr&token=h63OQoWehP&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro)](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro)

```diff
@@           Coverage Diff           @@
##             main     #907   +/-   ##
=======================================
  Coverage   90.95%   90.95%           
=======================================
  Files         125      125           
  Lines        7062     7062           
=======================================
  Hits         6423     6423           
  Misses        639      639           
```

| [Flag](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) | `90.95% <100.00%> (ø)` | |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Files](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) | Coverage Δ | |
|---|---|---|
| [src/neuroconv/tools/roiextractors/roiextractors.py](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907?src=pr&el=tree&filepath=src%2Fneuroconv%2Ftools%2Froiextractors%2Froiextractors.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro#diff-c3JjL25ldXJvY29udi90b29scy9yb2lleHRyYWN0b3JzL3JvaWV4dHJhY3RvcnMucHk=) | `88.02% <100.00%> (ø)` | |

</details>
codecov[bot]: ## [Codecov](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) Report
All modified and coverable lines are covered by tests :white_check_mark:
> Project coverage is 90.88%. Comparing base [(`5a65e74`)](https://app.codecov.io/gh/catalystneuro/neuroconv/commit/5a65e7417d90411bca3dce8312f8d6d483d30b05?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) to head [(`504493a`)](https://app.codecov.io/gh/catalystneuro/neuroconv/commit/504493a2f0d66c98f347418a1a3a5b66cb95f100?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro).
> Report is 5 commits behind head on main.

<details><summary>Additional details and impacted files</summary>


[![Impacted file tree graph](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907/graphs/tree.svg?width=650&height=150&src=pr&token=h63OQoWehP&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro)](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro)

```diff
@@            Coverage Diff             @@
##             main     #907      +/-   ##
==========================================
- Coverage   90.95%   90.88%   -0.07%     
==========================================
  Files         125      125              
  Lines        7062     7088      +26     
==========================================
+ Hits         6423     6442      +19     
- Misses        639      646       +7     
```

| [Flag](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) | Coverage Δ | |
|---|---|---|
| [unittests](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) | `90.88% <100.00%> (-0.07%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Files](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro) | Coverage Δ | |
|---|---|---|
| [src/neuroconv/tools/roiextractors/roiextractors.py](https://app.codecov.io/gh/catalystneuro/neuroconv/pull/907?src=pr&el=tree&filepath=src%2Fneuroconv%2Ftools%2Froiextractors%2Froiextractors.py&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=catalystneuro#diff-c3JjL25ldXJvY29udi90b29scy9yb2lleHRyYWN0b3JzL3JvaWV4dHJhY3RvcnMucHk=) | `88.02% <100.00%> (ø)` | |

</details>

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 31be213a..5dcccf0e 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -14,6 +14,7 @@
 
 ### Bug fixes
 * Fixed bug causing overwrite of NWB GUIDE watermark. [PR #890](https://github.com/catalystneuro/neuroconv/pull/890)
+* Change error trigger with warning trigger when adding both `OnePhotonSeries` and `TwoPhotonSeries` to the same file ([Issue #906](https://github.com/catalystneuro/neuroconv/issues/906)). [PR #907](https://github.com/catalystneuro/neuroconv/pull/907)
 
 
 
diff --git a/src/neuroconv/tools/roiextractors/roiextractors.py b/src/neuroconv/tools/roiextractors/roiextractors.py
index 87e36dba..08258f0e 100644
--- a/src/neuroconv/tools/roiextractors/roiextractors.py
+++ b/src/neuroconv/tools/roiextractors/roiextractors.py
@@ -399,10 +399,11 @@ def add_photon_series(
     metadata_copy = dict_deep_update(
         get_nwb_imaging_metadata(imaging, photon_series_type=photon_series_type), metadata_copy, append_list=False
     )
-    if photon_series_type == "TwoPhotonSeries":
-        assert (
-            "OnePhotonSeries" not in metadata_copy["Ophys"]
-        ), "Received metadata for 'OnePhotonSeries' but `photon_series_type` was not explicitly specified."
+
+    if photon_series_type == "TwoPhotonSeries" and "OnePhotonSeries" in metadata_copy["Ophys"]:
+        warn(
+            "Received metadata for both 'OnePhotonSeries' and 'TwoPhotonSeries', make sure photon_series_type is specified correctly."
+        )
 
     assert parent_container in [
         "acquisition",


Test patch:
diff --git a/tests/test_ophys/test_tools_roiextractors.py b/tests/test_ophys/test_tools_roiextractors.py
index 06673c1c..712cae87 100644
--- a/tests/test_ophys/test_tools_roiextractors.py
+++ b/tests/test_ophys/test_tools_roiextractors.py
@@ -1712,15 +1712,20 @@ class TestAddPhotonSeries(TestCase):
             )
 
     def test_add_photon_series_inconclusive_metadata(self):
-        """Test error is raised when `photon_series_type` specifies 'TwoPhotonSeries' but metadata contains 'OnePhotonSeries'."""
-        with self.assertRaisesWith(
-            AssertionError,
-            "Received metadata for 'OnePhotonSeries' but `photon_series_type` was not explicitly specified.",
-        ):
+        """Test warning is raised when `photon_series_type` specifies 'TwoPhotonSeries' but metadata contains also 'OnePhotonSeries'."""
+
+        exc_msg = "Received metadata for both 'OnePhotonSeries' and 'TwoPhotonSeries', make sure photon_series_type is specified correctly."
+        photon_series_metadata = deepcopy(self.one_photon_series_metadata)
+        photon_series_metadata["Ophys"].update(
+            TwoPhotonSeries=self.two_photon_series_metadata["Ophys"]["TwoPhotonSeries"]
+        )
+
+        with self.assertWarnsWith(warn_type=UserWarning, exc_msg=exc_msg):
             add_photon_series(
                 imaging=self.imaging_extractor,
                 nwbfile=self.nwbfile,
-                metadata=self.one_photon_series_metadata,
+                metadata=photon_series_metadata,
+                photon_series_type="TwoPhotonSeries",
             )
 
     def test_add_one_photon_series(self):