update sheet io for performer and factor value (#2033)

CHANGELOG.rst

@@ -65,6 +65,7 @@ Changed
     - Refactor ``SampleSheetAssayPluginPoint.get_assay_path()`` (#2016)
     - Return ``503`` in ``IrodsCollsCreateAPIView`` if project is locked (#1847)
     - Return ``503`` in ``IrodsDataRequestAcceptAPIView`` if project is locked (#1847)
+    - Remove length limitation from ``Process.performer`` (#1789, #1942, #2033)
 - **Taskflowbackend**
     - Refactor task tests (#2002)
     - Unify user name parameter naming in flows (#1653)

samplesheets/io.py

@@ -464,7 +464,7 @@ def _import_processes(
                 'protocol': protocol,
                 'assay': db_parent if isinstance(db_parent, Assay) else None,
                 'study': study,
-                'performer': p.performer,
+                'performer': ';'.join(p.performer) if p.performer else None,
                 'perform_date': p.date if p.date else None,
                 'array_design_ref': p.array_design_ref,
                 'first_dimension': (
@@ -539,7 +539,7 @@ def import_isa(
         :param save_isa: Save ISA-Tab as backup after importing (bool)
         :param from_template: Whether importing from a template (bool)
         :return: Investigation
-        :raise: SampleSheetExportException if critical warnings are raised
+        :raise: SampleSheetImportException if critical warnings are raised
         """
         t_start = time.time()
         logger.info('altamISA version: {}'.format(altamisa.__version__))
@@ -1070,7 +1070,11 @@ def _export_factor_vals(cls, factor_values):
         return tuple(
             isa_models.FactorValue(
                 name=k,
-                value=cls._export_val(v['value']),
+                value=(
+                    [cls._export_val(v['value'])]
+                    if not isinstance(v['value'], list)
+                    else cls._export_val(v['value'])
+                ),
                 unit=cls._export_val(v['unit']),
             )
             for k, v in factor_values.items()
@@ -1188,6 +1192,7 @@ def _export_processes(cls, processes):
                 perform_date = ''  # Empty string denotes an empty column
             else:
                 perform_date = p.perform_date
+            performer = p.performer.split(';') if p.performer else p.performer
             ret[p.unique_name] = isa_models.Process(
                 protocol_ref=(
                     p.protocol.name if p.protocol else PROTOCOL_UNKNOWN_NAME
@@ -1196,7 +1201,7 @@ def _export_processes(cls, processes):
                 name=p.name,
                 name_type=p.name_type,
                 date=perform_date,
-                performer=p.performer,
+                performer=performer,
                 parameter_values=cls._export_param_values(p.parameter_values),
                 comments=cls._export_comments(p.comments),
                 array_design_ref=p.array_design_ref,

samplesheets/migrations/0023_alter_process_performer.py

@@ -0,0 +1,20 @@
+# Generated by Django 4.2.16 on 2024-10-30 13:45
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ("samplesheets", "0022_update_igv_genome"),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name="process",
+            name="performer",
+            field=models.CharField(
+                blank=True, help_text="Process performer (optional)", null=True
+            ),
+        ),
+    ]

samplesheets/models.py

@@ -944,7 +944,6 @@ class Process(NodeMixin, BaseSampleSheet):
 
     #: Process performer (optional)
     performer = models.CharField(
-        max_length=DEFAULT_LENGTH,
         unique=False,
         blank=True,
         null=True,

samplesheets/tests/test_io.py

@@ -326,9 +326,8 @@ def setUp(self):
         )
         self.p_id = 'p{}'.format(self.project.pk)
 
-    def test_import_ref_val(self):
-        """Test _import_ref_val()"""
-        # Ontology value
+    def test_import_ref_val_ontology(self):
+        """Test _import_ref_val() with ontology value"""
         in_data = (
             self.isa_studies['s_BII-S-1.txt']
             .materials['{}-s0-source-culture1'.format(self.p_id)]
@@ -343,21 +342,15 @@ def test_import_ref_val(self):
         }
         self.assertEqual(out_data, expected)
 
-        # String value
-        in_data = (
-            self.isa_studies['s_BII-S-2.txt']
-            .materials[
-                '{}-s1-sample-NZ_4hrs_Grow1_Drug_Sample_1'.format(self.p_id)
-            ]
-            .factor_values[0]
-            .value
-        )
+    def test_import_ref_val_string(self):
+        """Test _import_ref_val() with string value"""
+        in_data = ' string '
         out_data = self.sheet_io._import_ref_val(in_data)
-        self.assertEqual(out_data, in_data)
+        self.assertEqual(out_data, in_data.strip())
 
-    def test_import_multi_val(self):
-        """Test _import_multi_val()"""
-        # List with a single ontology value (should return just a single dict)
+    def test_import_multi_val_single(self):
+        """Test _import_multi_val() with single ontology value"""
+        # Should return just a single dict
         in_data = (
             self.isa_studies['s_BII-S-1.txt']
             .materials['{}-s0-source-culture1'.format(self.p_id)]
@@ -372,9 +365,33 @@ def test_import_multi_val(self):
         }
         self.assertEqual(out_data, expected)
 
-        # TODO: List with multiple values (see issue #434)
+    def test_import_multi_val_list(self):
+        """Test _import_multi_val() with list of ontology values"""
+        in_data = [
+            isa_models.OntologyTermRef(
+                name='n1', accession='https://n1', ontology_name='TEST1'
+            ),
+            isa_models.OntologyTermRef(
+                name='n2', accession='https://n2', ontology_name='TEST2'
+            ),
+        ]
+        out_data = self.sheet_io._import_multi_val(in_data)
+        expected = [
+            {
+                'name': in_data[0].name,
+                'accession': in_data[0].accession,
+                'ontology_name': in_data[0].ontology_name,
+            },
+            {
+                'name': in_data[1].name,
+                'accession': in_data[1].accession,
+                'ontology_name': in_data[1].ontology_name,
+            },
+        ]
+        self.assertEqual(out_data, expected)
 
-        # Single ontology value
+    def test_import_multi_val_factor_value(self):
+        """Test _import_multi_val() with factor value"""
         in_data = (
             self.isa_studies['s_BII-S-1.txt']
             .materials['{}-s0-sample-C-0.07-aliquot9'.format(self.p_id)]
@@ -383,13 +400,14 @@ def test_import_multi_val(self):
         )
         out_data = self.sheet_io._import_multi_val(in_data)
         expected = {
-            'name': in_data.name,
-            'accession': in_data.accession,
-            'ontology_name': in_data.ontology_name,
+            'name': in_data[0].name,
+            'accession': in_data[0].accession,
+            'ontology_name': in_data[0].ontology_name,
         }
         self.assertEqual(out_data, expected)
 
-        # Ontology unit
+    def test_import_multi_val_ontology_unit(self):
+        """Test _import_multi_val() with ontology unit"""
         in_data = (
             self.isa_studies['s_BII-S-1.txt']
             .materials['{}-s0-sample-C-0.07-aliquot9'.format(self.p_id)]
@@ -595,8 +613,8 @@ def test_export_factors(self):
         }
         self.assertEqual(out_data, expected)
 
-    def test_export_factor_values(self):
-        """Test _export_factor_values()"""
+    def test_export_factor_vals(self):
+        """Test _export_factor_vals()"""
         study = self.investigation.studies.get(identifier='BII-S-1')
         in_data = study.materials.get(
             unique_name='{}-s0-sample-C-0.07-aliquot1'.format(self.p_id)
@@ -605,7 +623,7 @@ def test_export_factor_values(self):
         expected = tuple(
             isa_models.FactorValue(
                 name=k,
-                value=self.sheet_io._export_val(v['value']),
+                value=[self.sheet_io._export_val(v['value'])],
                 unit=self.sheet_io._export_val(v['unit']),
             )
             for k, v in in_data.items()