From cc5c3cd0c50a3a4d3ca12f5184a557859402ff14 Mon Sep 17 00:00:00 2001 From: Kaivan Kamali Date: Mon, 1 Mar 2021 14:31:15 -0500 Subject: [PATCH 1/6] Handle the case where column list is a comma-separated string --- lib/galaxy/tools/parameters/basic.py | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index 97d20f66d0c..b01ee101adb 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1361,9 +1361,12 @@ class ColumnListParameter(SelectToolParameter): legal_values = self.get_column_list(trans, other_values) value = other_values.get(self.name) - if value is not None and value not in legal_values and self.is_file_empty(trans, other_values): - value = value if isinstance(value, list) else [value] - legal_values.extend(value) + if value is not None: + # There are cases where 'value' is a string of comma separated values + # This ensures that it is converted into a list + value = value if isinstance(value, list) else value.split(",") + if not set(value).issubset(set(legal_values)) and self.is_file_empty(trans, other_values): + legal_values.extend(value) return set(legal_values) From 1673f9b16b67435476a5dd718893671440bb406a Mon Sep 17 00:00:00 2001 From: Kaivan Kamali Date: Mon, 1 Mar 2021 16:46:03 -0500 Subject: [PATCH 2/6] Using utility function per code review feedback. --- lib/galaxy/tools/parameters/basic.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index b01ee101adb..b305da4fcc1 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1362,9 +1362,9 @@ class ColumnListParameter(SelectToolParameter): value = other_values.get(self.name) if value is not None: - # There are cases where 'value' is a string of comma separated values - # This ensures that it is converted into a list - value = value if isinstance(value, list) else value.split(",") + # There are cases where 'value' is a string of comma separated values. This ensures + # that it is converted into a list, with extra whitespace around items removed. + value = util.listify(value, do_strip=True) if not set(value).issubset(set(legal_values)) and self.is_file_empty(trans, other_values): legal_values.extend(value) From 84647cbd0a05b8dd87a5ffe4662c033ad050b0ee Mon Sep 17 00:00:00 2001 From: Kaivan Kamali Date: Mon, 1 Mar 2021 17:36:42 -0500 Subject: [PATCH 3/6] Removed whitespace --- lib/galaxy/tools/parameters/basic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index b305da4fcc1..cec66a6eb14 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -1362,7 +1362,7 @@ class ColumnListParameter(SelectToolParameter): value = other_values.get(self.name) if value is not None: - # There are cases where 'value' is a string of comma separated values. This ensures + # There are cases where 'value' is a string of comma separated values. This ensures # that it is converted into a list, with extra whitespace around items removed. value = util.listify(value, do_strip=True) if not set(value).issubset(set(legal_values)) and self.is_file_empty(trans, other_values): From d56d228a7ae51324e9893b63598b5e6ec7e2a262 Mon Sep 17 00:00:00 2001 From: Marius van den Beek Date: Tue, 2 Mar 2021 15:50:37 +0000 Subject: [PATCH 4/6] Add regression test for allowing data column selection on empty files A test for https://github.com/galaxyproject/galaxy/pull/10981. --- lib/galaxy_test/api/test_workflows.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 4df8803c0bf..40119e12326 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -1193,6 +1193,26 @@ steps: invocation_id = self.__invoke_workflow(history_id, workflow_id, inputs) self.wait_for_invocation_and_jobs(history_id, workflow_id, invocation_id) + @skip_without_tool('column_param') + def test_empty_file_data_column_specified(self): + # Regression test for https://github.com/galaxyproject/galaxy/pull/10981 + with self.dataset_populator.test_history() as history_id: + self._run_jobs("""class: GalaxyWorkflow +steps: + empty_output: + tool_id: empty_output + outputs: + out_file1: + change_datatype: tabular + column_param: + tool_id: column_param + in: + input1: empty_output/out_file1 + state: + col: 2 + col_names: 'B' +""", history_id=history_id) + @skip_without_tool("mapper") @skip_without_tool("pileup") def test_workflow_metadata_validation_0(self): From 28c89ac9507593f488706d1eba5a8bf51b378d92 Mon Sep 17 00:00:00 2001 From: Kaivan Kamali Date: Tue, 2 Mar 2021 14:02:55 -0500 Subject: [PATCH 5/6] Added another test for when columns are a comma separated string --- lib/galaxy_test/api/test_workflows.py | 20 +++++++++ test/functional/tools/column_param_list.xml | 50 +++++++++++++++++++++ test/functional/tools/samples_tool_conf.xml | 1 + 3 files changed, 71 insertions(+) create mode 100644 test/functional/tools/column_param_list.xml diff --git a/lib/galaxy_test/api/test_workflows.py b/lib/galaxy_test/api/test_workflows.py index 40119e12326..9e609f39477 100644 --- a/lib/galaxy_test/api/test_workflows.py +++ b/lib/galaxy_test/api/test_workflows.py @@ -1213,6 +1213,26 @@ steps: col_names: 'B' """, history_id=history_id) + @skip_without_tool('column_param_list') + def test_comma_separated_columns(self): + # Regression test for https://github.com/galaxyproject/galaxy/pull/10981 + with self.dataset_populator.test_history() as history_id: + self._run_jobs("""class: GalaxyWorkflow +steps: + empty_output: + tool_id: empty_output + outputs: + out_file1: + change_datatype: tabular + column_param_list: + tool_id: column_param_list + in: + input1: empty_output/out_file1 + state: + col: '2,3' + col_names: 'B' +""", history_id=history_id) + @skip_without_tool("mapper") @skip_without_tool("pileup") def test_workflow_metadata_validation_0(self): diff --git a/test/functional/tools/column_param_list.xml b/test/functional/tools/column_param_list.xml new file mode 100644 index 00000000000..fba008db7c5 --- /dev/null +++ b/test/functional/tools/column_param_list.xml @@ -0,0 +1,50 @@ + + '$output1' && +echo "col $col" > '$output2' && +echo "col_names $col_names" >> '$output2' + ]]> + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/test/functional/tools/samples_tool_conf.xml b/test/functional/tools/samples_tool_conf.xml index b01cd8f42af..1a53ca33bf6 100644 --- a/test/functional/tools/samples_tool_conf.xml +++ b/test/functional/tools/samples_tool_conf.xml @@ -114,6 +114,7 @@ + From 423a95435ba125aec704072f01346188db614e0e Mon Sep 17 00:00:00 2001 From: Kaivan Kamali Date: Tue, 2 Mar 2021 17:35:42 -0500 Subject: [PATCH 6/6] Added a default value to fix a broken framework test. --- test/functional/tools/column_param_list.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/functional/tools/column_param_list.xml b/test/functional/tools/column_param_list.xml index fba008db7c5..a3c8d99e4f6 100644 --- a/test/functional/tools/column_param_list.xml +++ b/test/functional/tools/column_param_list.xml @@ -6,7 +6,7 @@ echo "col_names $col_names" >> '$output2' ]]> - +