From 4a8288b355f34955831aa0ffebb75d02a823c529 Mon Sep 17 00:00:00 2001 From: guerler Date: Thu, 31 Aug 2023 00:47:08 +0300 Subject: [PATCH] Rename map_over_type to collection_type to avoid confusion --- .../Form/Elements/FormData/FormData.test.js | 95 +++++++++++++------ .../Form/Elements/FormData/FormData.vue | 21 ++-- .../Form/Elements/FormData/types.ts | 4 +- lib/galaxy/tools/parameters/basic.py | 18 ++-- lib/galaxy/tools/parameters/meta.py | 2 +- lib/galaxy_test/api/test_tools.py | 18 ++-- .../selenium/test_workflow_editor.py | 14 +-- 7 files changed, 107 insertions(+), 65 deletions(-) diff --git a/client/src/components/Form/Elements/FormData/FormData.test.js b/client/src/components/Form/Elements/FormData/FormData.test.js index 2e00ef968286..9a825e91720a 100644 --- a/client/src/components/Form/Elements/FormData/FormData.test.js +++ b/client/src/components/Form/Elements/FormData/FormData.test.js @@ -29,7 +29,8 @@ const defaultOptions = { dce: [ { id: "dce1", name: "dceName1", src: "dce", is_dataset: true }, { id: "dce2", name: "dceName2", src: "dce" }, - { id: "dce3", name: "dceName3", src: "dce", map_over_type: "mapOverType" }, + { id: "dce3", name: "dceName3", src: "dce", collection_type: "collectionType" }, + { id: "dce4", name: "dceName4", src: "dce", is_dataset: true }, ], hda: [ { id: "hda1", hid: 1, name: "hdaName1", src: "hda" }, @@ -54,12 +55,12 @@ describe("FormData", () => { const value_0 = { batch: false, product: false, - values: [{ id: "hda3", src: "hda", map_over_type: null }], + values: [{ id: "hda3", src: "hda", collection_type: null }], }; const value_1 = { batch: false, product: false, - values: [{ id: "hda1", src: "hda", map_over_type: null }], + values: [{ id: "hda1", src: "hda", collection_type: null }], }; const options = wrapper.find(".btn-group").findAll("button"); expect(options.length).toBe(4); @@ -113,8 +114,8 @@ describe("FormData", () => { batch: false, product: false, values: [ - { id: "hda2", map_over_type: null, src: "hda" }, - { id: "hda3", map_over_type: null, src: "hda" }, + { id: "hda2", collection_type: null, src: "hda" }, + { id: "hda3", collection_type: null, src: "hda" }, ], }); expect(wrapper.emitted().input.length).toEqual(1); @@ -126,17 +127,25 @@ describe("FormData", () => { batch: false, product: false, values: [ - { id: "hda2", map_over_type: null, src: "hda" }, - { id: "hda3", map_over_type: null, src: "hda" }, + { id: "hda2", collection_type: null, src: "hda" }, + { id: "hda3", collection_type: null, src: "hda" }, ], }; expect(wrapper.emitted().input[0][0]).toEqual(value_0); await selectedValues.at(0).trigger("click"); - const value_1 = { batch: false, product: false, values: [{ id: "hda2", map_over_type: null, src: "hda" }] }; + const value_1 = { + batch: false, + product: false, + values: [{ id: "hda2", collection_type: null, src: "hda" }], + }; expect(wrapper.emitted().input[1][0]).toEqual(value_1); await wrapper.setProps({ value: value_1 }); await selectedValues.at(1).trigger("click"); - const value_2 = { batch: false, product: false, values: [{ id: "hda2", map_over_type: null, src: "hda" }] }; + const value_2 = { + batch: false, + product: false, + values: [{ id: "hda2", collection_type: null, src: "hda" }], + }; expect(wrapper.emitted().input[1][0]).toEqual(value_2); await wrapper.setProps({ value: value_2 }); expect(wrapper.emitted().input.length).toBe(3); @@ -148,7 +157,11 @@ describe("FormData", () => { value: { values: [{ id: "dce1", src: "dce" }] }, options: defaultOptions, }); - const value_0 = { batch: false, product: false, values: [{ id: "dce1", map_over_type: null, src: "dce" }] }; + const value_0 = { + batch: false, + product: false, + values: [{ id: "dce1", collection_type: null, src: "dce" }], + }; expect(wrapper.emitted().input[0][0]).toEqual(value_0); expect(wrapper.emitted().input.length).toEqual(1); const message = wrapper.findAll(".form-data-entry-label"); @@ -165,16 +178,16 @@ describe("FormData", () => { expect(wrapper.emitted().input.length).toEqual(2); }); - it("dataset collection as hdca without map_over_type", async () => { + it("dataset collection element as hdca without collection_type", async () => { const wrapper = createTarget({ value: { values: [{ id: "dce2", src: "dce" }] }, options: defaultOptions, }); - const value_0 = { batch: true, product: false, values: [{ id: "dce2", map_over_type: null, src: "dce" }] }; + const value_0 = { batch: true, product: false, values: [{ id: "dce2", collection_type: null, src: "dce" }] }; expect(wrapper.emitted().input[0][0]).toEqual(value_0); }); - it("dataset collection as hdca with map_over_type", async () => { + it("dataset collection element as hdca mapped to batch field", async () => { const wrapper = createTarget({ value: { values: [{ id: "dce3", src: "dce" }] }, options: defaultOptions, @@ -182,17 +195,45 @@ describe("FormData", () => { const value_0 = { batch: true, product: false, - values: [{ id: "dce3", map_over_type: "mapOverType", src: "dce" }], + values: [{ id: "dce3", collection_type: "collectionType", src: "dce" }], + }; + expect(wrapper.emitted().input[0][0]).toEqual(value_0); + }); + + it("dataset collection element as hdca mapped to non-batch field", async () => { + const wrapper = createTarget({ + type: "data_collection", + value: { values: [{ id: "dce3", src: "dce" }] }, + options: defaultOptions, + }); + const value_0 = { + batch: false, + product: false, + values: [{ id: "dce3", collection_type: "collectionType", src: "dce" }], + }; + expect(wrapper.emitted().input[0][0]).toEqual(value_0); + }); + + it("dataset collection mapped to non-batch field", async () => { + const wrapper = createTarget({ + type: "data_collection", + value: { values: [{ id: "hdca4", src: "hdca" }] }, + options: defaultOptions, + }); + const value_0 = { + batch: false, + product: false, + values: [{ id: "hdca4", collection_type: null, src: "hdca" }], }; expect(wrapper.emitted().input[0][0]).toEqual(value_0); }); - it("multiple dataset collection values with varying map_over_type", async () => { + it("multiple dataset collection elements (as hdas)", async () => { const wrapper = createTarget({ value: { values: [ - { id: "dce2", src: "dce" }, - { id: "dce3", src: "dce" }, + { id: "dce1", src: "dce" }, + { id: "dce4", src: "dce" }, ], }, options: defaultOptions, @@ -201,8 +242,8 @@ describe("FormData", () => { batch: true, product: false, values: [ - { id: "dce2", map_over_type: null, src: "dce" }, - { id: "dce3", map_over_type: "mapOverType", src: "dce" }, + { id: "dce1", collection_type: null, src: "dce" }, + { id: "dce4", collection_type: null, src: "dce" }, ], }; expect(wrapper.emitted().input[0][0]).toEqual(value_0); @@ -219,7 +260,7 @@ describe("FormData", () => { expect(wrapper.emitted().input[1][0]).toEqual({ batch: true, product: false, - values: [{ id: "hdca4", map_over_type: null, src: "hdca" }], + values: [{ id: "hdca4", collection_type: null, src: "hdca" }], }); eventStore.setDragData({ id: "hda2", history_content_type: "dataset" }); dispatchEvent(wrapper, "dragenter"); @@ -227,7 +268,7 @@ describe("FormData", () => { expect(wrapper.emitted().input[2][0]).toEqual({ batch: false, product: false, - values: [{ id: "hda2", map_over_type: null, src: "hda" }], + values: [{ id: "hda2", collection_type: null, src: "hda" }], }); }); @@ -240,7 +281,7 @@ describe("FormData", () => { expect(wrapper.emitted().input[0][0]).toEqual({ batch: false, product: false, - values: [{ id: "hda3", map_over_type: null, src: "hda" }], + values: [{ id: "hda3", collection_type: null, src: "hda" }], }); const noCheckLinked = wrapper.find("input[type='checkbox']"); expect(noCheckLinked.exists()).toBeFalsy(); @@ -248,7 +289,7 @@ describe("FormData", () => { expect(wrapper.emitted().input[1][0]).toEqual({ batch: true, product: false, - values: [{ id: "hda3", map_over_type: null, src: "hda" }], + values: [{ id: "hda3", collection_type: null, src: "hda" }], }); const checkLinked = wrapper.find("input[type='checkbox']"); expect(wrapper.find(".custom-switch span").text()).toBe( @@ -262,11 +303,11 @@ describe("FormData", () => { expect(wrapper.emitted().input[2][0]).toEqual({ batch: true, product: true, - values: [{ id: "hda3", map_over_type: null, src: "hda" }], + values: [{ id: "hda3", collection_type: null, src: "hda" }], }); }); - it("match variation on initial value", async () => { + it("match dataset collection on initial value", async () => { const wrapper = createTarget({ value: { values: [{ id: "hdca4", src: "hdca" }], @@ -283,7 +324,7 @@ describe("FormData", () => { expect(wrapper.emitted().input[i][0]).toEqual({ batch: false, product: false, - values: [{ id: "hdca4", map_over_type: null, src: "hdca" }], + values: [{ id: "hdca4", collection_type: null, src: "hdca" }], }); } expect(wrapper.emitted().input.length).toEqual(2); @@ -295,7 +336,7 @@ describe("FormData", () => { expect(wrapper.emitted().input[2][0]).toEqual({ batch: false, product: false, - values: [{ id: "hda3", map_over_type: null, src: "hda" }], + values: [{ id: "hda3", collection_type: null, src: "hda" }], }); const newSelectedValues = wrapper.findAll(SELECTED_VALUE); expect(newSelectedValues.at(0).text()).toBe("3: hdaName3"); diff --git a/client/src/components/Form/Elements/FormData/FormData.vue b/client/src/components/Form/Elements/FormData/FormData.vue index 3e73abbb2eb5..64a25584bc84 100644 --- a/client/src/components/Form/Elements/FormData/FormData.vue +++ b/client/src/components/Form/Elements/FormData/FormData.vue @@ -206,7 +206,6 @@ function handleIncoming(incoming: Record, partial = true) { const incomingValues: Array = []; values.forEach((v) => { // Map incoming objects to data option values - v.id = v.element_id || v.id; const newHid = v.hid; const newId = v.id; const newName = v.name ? v.name : newId; @@ -381,7 +380,7 @@ function setValue(val: Array | DataOption | null) { if (val) { const values = Array.isArray(val) ? val : [val]; if (variant.value && values.length > 0 && values[0]) { - const hasMapOverType = values.find((v) => !!v.map_over_type); + const hasMapOverType = values.find((v) => !!v.collection_type); const isMultiple = values.length > 1; // Determine source representation @@ -403,14 +402,16 @@ function setValue(val: Array | DataOption | null) { let batch: string = BATCH.DISABLED; if (variantIndex >= 0) { const variantDetails = variant.value[variantIndex]; - if ((isLDDA.value || isDCE.value) && variantDetails && variantDetails.batch) { - batch = variantDetails.batch; - } else { - // Switch to another field type if source differs from current field - if (currentVariant.value && currentVariant.value.src !== sourceType) { - currentField.value = variantIndex; + if (variantDetails) { + if ((isLDDA.value || isDCE.value) && variantDetails.batch) { + batch = variantDetails.batch; + } else { + // Switch to another field type if source differs from current field + if (currentVariant.value && currentVariant.value.src !== sourceType) { + currentField.value = variantIndex; + } + batch = (currentVariant.value && currentVariant.value.batch) || BATCH.DISABLED; } - batch = (currentVariant.value && currentVariant.value.batch) || BATCH.DISABLED; } } @@ -421,7 +422,7 @@ function setValue(val: Array | DataOption | null) { values: values.map((entry) => ({ id: entry.id, src: entry.src, - map_over_type: entry.map_over_type || null, + collection_type: entry.collection_type || null, })), }); } diff --git a/client/src/components/Form/Elements/FormData/types.ts b/client/src/components/Form/Elements/FormData/types.ts index 95a1c6b8002a..85a798a0eab3 100644 --- a/client/src/components/Form/Elements/FormData/types.ts +++ b/client/src/components/Form/Elements/FormData/types.ts @@ -1,9 +1,9 @@ export interface DataOption { id: string; + collection_type?: string; + hid: number; is_dataset?: boolean; keep: boolean; - hid: number; - map_over_type?: string; name: string; src: string; tags: Array; diff --git a/lib/galaxy/tools/parameters/basic.py b/lib/galaxy/tools/parameters/basic.py index ae0598d8acdf..327d502f00d9 100644 --- a/lib/galaxy/tools/parameters/basic.py +++ b/lib/galaxy/tools/parameters/basic.py @@ -2289,7 +2289,7 @@ def to_dict(self, trans, other_values=None): multiple = self.multiple # build and append a new select option - def append(list, hda, name, src, keep=False, subcollection_type=None): + def append(list, hda, name, src, keep=False, collection_type=None): value = { "id": trans.security.encode_id(hda.id), "hid": hda.hid if hda.hid is not None else -1, @@ -2298,8 +2298,8 @@ def append(list, hda, name, src, keep=False, subcollection_type=None): "src": src, "keep": keep, } - if subcollection_type: - value["map_over_type"] = subcollection_type + if collection_type: + value["collection_type"] = collection_type return list.append(value) def append_dce(dce): @@ -2364,18 +2364,18 @@ def append_ldda(ldda): for hdca in history.active_visible_dataset_collections: match = dataset_collection_matcher.hdca_match(hdca) if match: - subcollection_type = None + collection_type = None if multiple and hdca.collection.collection_type != "list": collection_type_description = self._history_query(trans).can_map_over(hdca) if collection_type_description: - subcollection_type = collection_type_description.collection_type + collection_type = collection_type_description.collection_type else: continue name = hdca.name if match.implicit_conversion: name = f"{name} (with implicit datatype conversion)" - append(d["options"]["hdca"], hdca, name, "hdca", subcollection_type=subcollection_type) + append(d["options"]["hdca"], hdca, name, "hdca", collection_type=collection_type) continue # sort both lists @@ -2519,7 +2519,7 @@ def to_dict(self, trans, other_values=None): # append DCE if isinstance(other_values.get(self.name), DatasetCollectionElement): dce = other_values[self.name] - d["options"]["hdca"].append( + d["options"]["dce"].append( { "id": trans.security.encode_id(dce.id), "hid": -1, @@ -2546,18 +2546,18 @@ def to_dict(self, trans, other_values=None): # append matching subcollections for hdca, implicit_conversion in self.match_multirun_collections(trans, history, dataset_collection_matcher): - subcollection_type = self._history_query(trans).can_map_over(hdca).collection_type + collection_type = self._history_query(trans).can_map_over(hdca).collection_type name = hdca.name if implicit_conversion: name = f"{name} (with implicit datatype conversion)" d["options"]["hdca"].append( { "id": trans.security.encode_id(hdca.id), + "collection_type": collection_type, "hid": hdca.hid, "name": name, "src": "hdca", "tags": [t.user_tname if not t.value else f"{t.user_tname}:{t.value}" for t in hdca.tags], - "map_over_type": subcollection_type, } ) diff --git a/lib/galaxy/tools/parameters/meta.py b/lib/galaxy/tools/parameters/meta.py index a9f47ac0eae0..ec69a59b86e5 100644 --- a/lib/galaxy/tools/parameters/meta.py +++ b/lib/galaxy/tools/parameters/meta.py @@ -260,7 +260,7 @@ def __expand_collection_parameter(trans, input_key, incoming_val, collections_to if src != "hdca": raise exceptions.ToolMetaParameterException(f"Invalid dataset collection source type {src}") encoded_hdc_id = incoming_val["id"] - subcollection_type = incoming_val.get("map_over_type", None) + subcollection_type = incoming_val.get("collection_type", None) except TypeError: encoded_hdc_id = incoming_val subcollection_type = None diff --git a/lib/galaxy_test/api/test_tools.py b/lib/galaxy_test/api/test_tools.py index 2b8620d964fa..2ecec0a752ef 100644 --- a/lib/galaxy_test/api/test_tools.py +++ b/lib/galaxy_test/api/test_tools.py @@ -609,7 +609,7 @@ def test_unzip_nested(self): inputs = { "input": { "batch": True, - "values": [{"src": "hdca", "map_over_type": "paired", "id": hdca_id}], + "values": [{"src": "hdca", "collection_type": "paired", "id": hdca_id}], } } response = self._run("__UNZIP_COLLECTION__", history_id, inputs, assert_ok=True) @@ -795,7 +795,7 @@ def _run_filter(self, history_id, failed_hdca_id, batch=False): inputs = { "input": { "batch": batch, - "values": [{"map_over_type": "list:paired", "src": "hdca", "id": failed_hdca_id}], + "values": [{"collection_type": "list:paired", "src": "hdca", "id": failed_hdca_id}], }, } else: @@ -1691,7 +1691,7 @@ def test_map_over_with_output_format_actions(self, history_id): @skip_without_tool("output_action_change_format_paired") def test_map_over_with_nested_paired_output_format_actions(self, history_id): hdca_id = self.__build_nested_list(history_id) - inputs = {"input": {"batch": True, "values": [dict(map_over_type="paired", src="hdca", id=hdca_id)]}} + inputs = {"input": {"batch": True, "values": [dict(collection_type="paired", src="hdca", id=hdca_id)]}} create = self._run("output_action_change_format_paired", history_id, inputs).json() outputs = create["outputs"] jobs = create["jobs"] @@ -2109,7 +2109,7 @@ def test_map_over_nested_collections(self, history_id): def test_paired_input_map_over_nested_collections(self, history_id): hdca_id = self.__build_nested_list(history_id) inputs = { - "input1": {"batch": True, "values": [dict(map_over_type="paired", src="hdca", id=hdca_id)]}, + "input1": {"batch": True, "values": [dict(collection_type="paired", src="hdca", id=hdca_id)]}, } self.dataset_populator.wait_for_history(history_id, assert_ok=True) create = self._run("collection_paired_structured_like", history_id, inputs, assert_ok=True) @@ -2127,7 +2127,7 @@ def test_paired_input_conditional_map_over_nested_collections(self, history_id): hdca_id = self.__build_nested_list(history_id) inputs = { "cond|cond_param": "paired", - "cond|input1": {"batch": True, "values": [dict(map_over_type="paired", src="hdca", id=hdca_id)]}, + "cond|input1": {"batch": True, "values": [dict(collection_type="paired", src="hdca", id=hdca_id)]}, } self.dataset_populator.wait_for_history(history_id, assert_ok=True) create = self._run("collection_paired_conditional_structured_like", history_id, inputs, assert_ok=True) @@ -2440,7 +2440,7 @@ def test_implicit_reduce_with_mapping(self): "f1": {"src": "hdca", "id": hdca1_id}, "f2": { "batch": True, - "values": [{"src": "hdca", "map_over_type": "list", "id": hdca2_id}], + "values": [{"src": "hdca", "collection_type": "list", "id": hdca2_id}], }, } create = self._run("multi_data_param", history_id, inputs, assert_ok=True) @@ -2569,7 +2569,7 @@ def test_subcollection_mapping(self): inputs = { "f1": { "batch": True, - "values": [{"src": "hdca", "map_over_type": "paired", "id": hdca_list_id}], + "values": [{"src": "hdca", "collection_type": "paired", "id": hdca_list_id}], } } self._check_simple_subcollection_mapping(history_id, inputs) @@ -2598,7 +2598,7 @@ def test_combined_mapping_and_subcollection_mapping(self): inputs = { "f1": { "batch": True, - "values": [{"src": "hdca", "map_over_type": "paired", "id": nested_list_id}], + "values": [{"src": "hdca", "collection_type": "paired", "id": nested_list_id}], }, "f2": { "batch": True, @@ -2975,7 +2975,7 @@ def test_deferred_map_over_nested_collections(self, history_id): dataset_collection = self.dataset_collection_populator.wait_for_fetched_collection(fetch_response) hdca_id = dataset_collection["id"] inputs = { - "input1": {"batch": True, "values": [dict(map_over_type="paired", src="hdca", id=hdca_id)]}, + "input1": {"batch": True, "values": [dict(collection_type="paired", src="hdca", id=hdca_id)]}, } self.dataset_populator.wait_for_history(history_id, assert_ok=True) create = self._run("collection_paired_structured_like", history_id, inputs, assert_ok=True) diff --git a/lib/galaxy_test/selenium/test_workflow_editor.py b/lib/galaxy_test/selenium/test_workflow_editor.py index 7156a33962fb..fc5eae463bd5 100644 --- a/lib/galaxy_test/selenium/test_workflow_editor.py +++ b/lib/galaxy_test/selenium/test_workflow_editor.py @@ -1076,7 +1076,7 @@ def test_map_over_output_indicator(self): self.workflow_editor_destroy_connection("filter#how|filter_source") self.assert_node_output_is("filter#output_filtered", "list") - def assert_node_output_is(self, label: str, output_type: str, map_over_type: Optional[str] = None): + def assert_node_output_is(self, label: str, output_type: str, collection_type: Optional[str] = None): editor = self.components.workflow_editor node_label, output_name = label.split("#") node = editor.node._(label=node_label) @@ -1085,18 +1085,18 @@ def assert_node_output_is(self, label: str, output_type: str, map_over_type: Opt self.hover_over(output_element) element = self.components._.tooltip_inner.wait_for_present() assert f"output is {output_type}" in element.text, element.text - if map_over_type is None: + if collection_type is None: assert "mapped-over" not in element.text else: fragment = " and mapped-over to produce a " - if map_over_type == "list:paired": + if collection_type == "list:paired": fragment += "list of pairs dataset collection" - elif map_over_type == "list:list": + elif collection_type == "list:list": fragment += "list of lists dataset collection" - elif map_over_type.count(":") > 1: - fragment += f"dataset collection with {map_over_type.count(':') + 1} levels of nesting" + elif collection_type.count(":") > 1: + fragment += f"dataset collection with {collection_type.count(':') + 1} levels of nesting" else: - fragment += f"{map_over_type}" + fragment += f"{collection_type}" assert fragment in element.text self.click_center()