Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
import { describe, expect, it } from "vitest";

import {
ANY_COLLECTION_TYPE_DESCRIPTION,
CollectionTypeDescription,
NULL_COLLECTION_TYPE_DESCRIPTION,
} from "./collectionTypeDescription";

const ct = (collectionType: string) => new CollectionTypeDescription(collectionType);

describe("accepts (asymmetric subtype check)", () => {
it("same type accepts itself", () => {
expect(ct("list").accepts(ct("list"))).toBe(true);
expect(ct("paired").accepts(ct("paired"))).toBe(true);
expect(ct("list:paired").accepts(ct("list:paired"))).toBe(true);
});

it("paired_or_unpaired requirement is satisfied by paired candidate (not vice versa)", () => {
expect(ct("paired_or_unpaired").accepts(ct("paired"))).toBe(true);
expect(ct("paired").accepts(ct("paired_or_unpaired"))).toBe(false);
expect(ct("list:paired_or_unpaired").accepts(ct("list:paired"))).toBe(true);
expect(ct("list:paired").accepts(ct("list:paired_or_unpaired"))).toBe(false);
});

it("list requirement is satisfied by sample_sheet candidate (not vice versa)", () => {
expect(ct("list").accepts(ct("sample_sheet"))).toBe(true);
expect(ct("sample_sheet").accepts(ct("list"))).toBe(false);
expect(ct("list:paired").accepts(ct("sample_sheet:paired"))).toBe(true);
expect(ct("sample_sheet:paired").accepts(ct("list:paired"))).toBe(false);
});

it("disjoint types do not accept each other", () => {
expect(ct("paired").accepts(ct("list"))).toBe(false);
expect(ct("list").accepts(ct("paired"))).toBe(false);
});

it("ANY accepts any non-null collection", () => {
expect(ANY_COLLECTION_TYPE_DESCRIPTION.accepts(ct("list"))).toBe(true);
expect(ANY_COLLECTION_TYPE_DESCRIPTION.accepts(NULL_COLLECTION_TYPE_DESCRIPTION)).toBe(false);
});

it("NULL accepts nothing", () => {
expect(NULL_COLLECTION_TYPE_DESCRIPTION.accepts(ct("list"))).toBe(false);
expect(NULL_COLLECTION_TYPE_DESCRIPTION.accepts(ANY_COLLECTION_TYPE_DESCRIPTION)).toBe(false);
});
});

describe("compatible (symmetric sibling-matching check)", () => {
it("is symmetric for subtype pairs", () => {
// sample_sheet <-> list
expect(ct("list").compatible(ct("sample_sheet"))).toBe(true);
expect(ct("sample_sheet").compatible(ct("list"))).toBe(true);
expect(ct("list:paired").compatible(ct("sample_sheet:paired"))).toBe(true);
expect(ct("sample_sheet:paired").compatible(ct("list:paired"))).toBe(true);

// paired <-> paired_or_unpaired
expect(ct("paired").compatible(ct("paired_or_unpaired"))).toBe(true);
expect(ct("paired_or_unpaired").compatible(ct("paired"))).toBe(true);
expect(ct("list:paired").compatible(ct("list:paired_or_unpaired"))).toBe(true);
expect(ct("list:paired_or_unpaired").compatible(ct("list:paired"))).toBe(true);
});

it("same type is compatible with itself", () => {
expect(ct("list").compatible(ct("list"))).toBe(true);
expect(ct("paired").compatible(ct("paired"))).toBe(true);
});

it("disjoint types are not compatible (either order)", () => {
expect(ct("paired").compatible(ct("list"))).toBe(false);
expect(ct("list").compatible(ct("paired"))).toBe(false);
expect(ct("list:paired").compatible(ct("list:list"))).toBe(false);
expect(ct("list:list").compatible(ct("list:paired"))).toBe(false);
});
});
Original file line number Diff line number Diff line change
@@ -1,4 +1,21 @@
/* classes for reasoning about collection map over state
/* Classes for reasoning about collection map-over state and the
compatibility algebra.

Two operations on collection types:
- accepts(other): asymmetric. input_type.accepts(output_type) is true
iff an output of type other can be connected to an input slot of
type this. Used at workflow-editor edge validation.
- compatible(other): symmetric. True iff this and other match such
that they could drive a common map-over over sibling inputs of one
tool. Used for sibling map-over state checks (neither side is the
input slot), e.g. mappingConstraints in terminals.ts.

Mirrors the Python implementation in
lib/galaxy/model/dataset_collections/type_description.py — keep in sync.
See lib/galaxy/model/dataset_collections/types/collection_semantics.yml
"Type Compatibility Algebra" for the lattice diagram and worked examples.

Type variants:
null: not a collection?
NULL_COLLECTION_TYPE_DESCRIPTION: also not a collection. Is there any difference with null ?
ANY_COLLECTION_TYPE_DESCRIPTION: collection, but will never be mapped over (input has no collection_type)
Expand All @@ -9,7 +26,8 @@ export interface CollectionTypeDescriptor {
isCollection: boolean;
collectionType: string | null;
rank: number;
canMatch(other: CollectionTypeDescriptor): boolean;
accepts(other: CollectionTypeDescriptor): boolean;
compatible(other: CollectionTypeDescriptor): boolean;
canMapOver(other: CollectionTypeDescriptor): boolean;
append(other: CollectionTypeDescriptor): CollectionTypeDescriptor;
equal(other: CollectionTypeDescriptor | null): boolean;
Expand All @@ -21,7 +39,10 @@ export const NULL_COLLECTION_TYPE_DESCRIPTION: CollectionTypeDescriptor = {
isCollection: false,
collectionType: null,
rank: 0,
canMatch: function (_other) {
accepts: function (_other) {
return false;
},
compatible: function (_other) {
return false;
},
canMapOver: function () {
Expand All @@ -45,7 +66,10 @@ export const ANY_COLLECTION_TYPE_DESCRIPTION: CollectionTypeDescriptor = {
isCollection: true,
collectionType: "any",
rank: -1,
canMatch: function (other) {
accepts: function (other) {
return NULL_COLLECTION_TYPE_DESCRIPTION !== other;
},
compatible: function (other) {
return NULL_COLLECTION_TYPE_DESCRIPTION !== other;
},
canMapOver: function () {
Expand Down Expand Up @@ -95,14 +119,30 @@ export class CollectionTypeDescription implements CollectionTypeDescriptor {
}
return new CollectionTypeDescription(`${this.collectionType}:${other.collectionType}`);
}
canMatch(other: CollectionTypeDescriptor) {
/**
* Asymmetric direct-edge check: does an input slot of type ``this``
* accept an output of type ``other``? Convention:
* ``input_type.accepts(output_type)``. Used at workflow-editor edge
* validation.
*/
accepts(other: CollectionTypeDescriptor) {
const otherCollectionType = other.collectionType;
if (other === NULL_COLLECTION_TYPE_DESCRIPTION) {
return false;
}
if (other === ANY_COLLECTION_TYPE_DESCRIPTION) {
return true;
}
// sample_sheet asymmetry: a sample_sheet input needs column metadata
// that a plain-list output cannot provide. Check raw types before
// normalization (which otherwise equates the two).
if (
this.collectionType.startsWith("sample_sheet") &&
otherCollectionType &&
!otherCollectionType.startsWith("sample_sheet")
) {
return false;
}
const normalizedThis = normalizeCollectionType(this.collectionType);
const normalizedOther = otherCollectionType ? normalizeCollectionType(otherCollectionType) : null;
if (normalizedOther === "paired" && normalizedThis == "paired_or_unpaired") {
Expand All @@ -120,10 +160,25 @@ export class CollectionTypeDescription implements CollectionTypeDescriptor {
}
return normalizedOther == normalizedThis;
}
/**
* Symmetric sibling-matching check: do ``this`` and ``other`` match
* such that they could drive a common map-over over sibling inputs of
* a single tool? Used for sibling map-over state checks
* (terminals.ts mappingConstraints).
*/
compatible(other: CollectionTypeDescriptor) {
return this.accepts(other) || other.accepts(this);
}
canMapOver(other: CollectionTypeDescriptor) {
if (!other.collectionType || other.collectionType === "any") {
return false;
}
// sample_sheet asymmetry: a sample_sheet input requires sample_sheet
// column metadata; a plain-list output cannot be mapped over it.
// ``this`` is the output, ``other`` is the input.
if (other.collectionType.startsWith("sample_sheet") && !this.collectionType.startsWith("sample_sheet")) {
return false;
}
const normalizedThis = normalizeCollectionType(this.collectionType);
const normalizedOther = normalizeCollectionType(other.collectionType);
if (this.rank <= other.rank) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -700,7 +700,7 @@ describe("canAccept", () => {
expect(dataIn.canAccept(collectionOut).canAccept).toBe(true);
expect(dataIn.mapOver).toEqual(NULL_COLLECTION_TYPE_DESCRIPTION);
});
it("accepts sample_sheet -> list connection (canMatch)", () => {
it("accepts sample_sheet -> list connection (accepts)", () => {
const collectionOut = terminals["sample_sheet input"]!["output"] as OutputCollectionTerminal;
const dataIn = terminals["list collection input"]!["input1"] as InputCollectionTerminal;
expect(dataIn.mapOver).toBe(NULL_COLLECTION_TYPE_DESCRIPTION);
Expand All @@ -716,7 +716,7 @@ describe("canAccept", () => {
dataIn.connect(collectionOut);
expect(dataIn.mapOver).toEqual({ collectionType: "sample_sheet", isCollection: true, rank: 1 });
});
it("accepts sample_sheet:paired -> list:paired connection (canMatch)", () => {
it("accepts sample_sheet:paired -> list:paired connection (accepts)", () => {
const collectionOut = terminals["sample_sheet:paired input"]!["output"] as OutputCollectionTerminal;
const dataIn = terminals["list:paired collection input"]!["input1"] as InputCollectionTerminal;
expect(dataIn.mapOver).toBe(NULL_COLLECTION_TYPE_DESCRIPTION);
Expand All @@ -740,7 +740,7 @@ describe("canAccept", () => {
dataIn.connect(collectionOut);
expect(dataIn.mapOver).toEqual({ collectionType: "sample_sheet", isCollection: true, rank: 1 });
});
it("accepts sample_sheet:paired_or_unpaired -> list:paired_or_unpaired connection (canMatch)", () => {
it("accepts sample_sheet:paired_or_unpaired -> list:paired_or_unpaired connection (accepts)", () => {
const collectionOut = terminals["sample_sheet:paired_or_unpaired input"]!["output"] as OutputCollectionTerminal;
const dataIn = terminals["list:paired_or_unpaired collection input"]!["f1"] as InputTerminal;
expect(dataIn.mapOver).toBe(NULL_COLLECTION_TYPE_DESCRIPTION);
Expand Down
53 changes: 25 additions & 28 deletions client/src/components/Workflow/Editor/modules/terminals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -501,19 +501,21 @@ export class InputTerminal extends BaseInputTerminal {
);
}
}
if (mapOver.isCollection && mapOver.canMatch(otherCollectionType)) {
if (mapOver.isCollection && mapOver.accepts(otherCollectionType)) {
return this._producesAcceptableDatatypeAndOptionalness(other);
} else if (
this.multiple &&
new CollectionTypeDescription("list").append(this.mapOver).canMatch(otherCollectionType)
new CollectionTypeDescription("list").append(this.mapOver).accepts(otherCollectionType)
) {
// This handles the special case of a list input being connected to a multiple="true" data input.
// Nested lists would be correctly mapped over by the above condition.
return this._producesAcceptableDatatypeAndOptionalness(other);
} else {
// Need to check if this would break constraints...
// Sibling map-over states: use symmetric ``compatible`` so order
// of arrival of sibling inputs doesn't change the answer.
const mappingConstraints = this._mappingConstraints();
if (mappingConstraints.every(otherCollectionType.canMatch.bind(otherCollectionType))) {
if (mappingConstraints.every((constraint) => constraint.compatible(otherCollectionType))) {
return this._producesAcceptableDatatypeAndOptionalness(other);
} else {
if (mapOver.isCollection) {
Expand Down Expand Up @@ -619,8 +621,8 @@ export class InputCollectionTerminal extends BaseInputTerminal {
}
_effectiveMapOver(otherCollectionType: CollectionTypeDescriptor) {
const collectionTypes = this.collectionTypes;
const canMatch = collectionTypes.some((collectionType) => collectionType.canMatch(otherCollectionType));
if (!canMatch) {
const directlyAccepted = collectionTypes.some((collectionType) => collectionType.accepts(otherCollectionType));
if (!directlyAccepted) {
for (const collectionTypeIndex in collectionTypes) {
const collectionType = collectionTypes[collectionTypeIndex]!;

Expand All @@ -643,12 +645,16 @@ export class InputCollectionTerminal extends BaseInputTerminal {
if (otherCollectionType.isCollection) {
const effectiveCollectionTypes = this._effectiveCollectionTypes();
const mapOver = this.mapOver;
const canMatch = effectiveCollectionTypes.some((effectiveCollectionType, i) => {
if (!effectiveCollectionType.canMatch(otherCollectionType)) {
// Defense-in-depth: ``accepts`` carries the sample_sheet asymmetry
// guard, but only when the receiver type itself starts with
// "sample_sheet". A non-null ``localMapOver`` could in principle
// produce an effective type like "list:sample_sheet" that hides
// the guard from ``accepts``. Re-check against the raw declared
// input type to be safe — matches the structure HEAD had inline.
const accepted = effectiveCollectionTypes.some((effectiveCollectionType, i) => {
if (!effectiveCollectionType.accepts(otherCollectionType)) {
return false;
}
// sample_sheet asymmetry: sample_sheet input requires sample_sheet output,
// but sample_sheet output can satisfy list input.
const rawInputType = this.collectionTypes[i]?.collectionType;
if (
rawInputType?.startsWith("sample_sheet") &&
Expand All @@ -658,7 +664,7 @@ export class InputCollectionTerminal extends BaseInputTerminal {
}
return true;
});
if (canMatch) {
if (accepted) {
// Only way a direct match...
return this._producesAcceptableDatatypeAndOptionalness(other);
// Otherwise we need to mapOver
Expand All @@ -675,26 +681,14 @@ export class InputCollectionTerminal extends BaseInputTerminal {
"Can't map over this input with output collection type - this step has outputs defined constraining the mapping of this tool. Disconnect outputs and retry.",
);
}
} else if (
this.collectionTypes.some((collectionType) => {
if (!otherCollectionType.canMapOver(collectionType)) {
return false;
}
// sample_sheet asymmetry: same as canMatch guard above
if (
collectionType.collectionType?.startsWith("sample_sheet") &&
!otherCollectionType.collectionType?.startsWith("sample_sheet")
) {
return false;
}
return true;
})
) {
} else if (this.collectionTypes.some((collectionType) => otherCollectionType.canMapOver(collectionType))) {
// we're not mapped over - but hey maybe we could be... lets check.
const effectiveMapOver = this._effectiveMapOver(otherCollectionType);
// Need to check if this would break constraints...
// Sibling map-over states: use symmetric ``compatible`` so order
// of arrival of sibling inputs doesn't change the answer.
const mappingConstraints = this._mappingConstraints();
if (mappingConstraints.every((d) => effectiveMapOver.canMatch(d))) {
if (mappingConstraints.every((d) => d.compatible(effectiveMapOver))) {
return this._producesAcceptableDatatypeAndOptionalness(other);
} else {
return new ConnectionAcceptable(
Expand Down Expand Up @@ -857,10 +851,13 @@ export class OutputCollectionTerminal extends BaseOutputTerminal {
const otherCollectionType = inputTerminal._otherCollectionType(outputTerminal);
// we need to find which of the possible input collection types is connected
if ("collectionTypes" in inputTerminal) {
// collection_type_source must point at input collection terminal
// collection_type_source must point at input collection terminal.
// Direction here is input_type.accepts(output_type): the
// receiver is the declared input collection type; the
// argument is the connected output's shape.
const connectedCollectionType = inputTerminal.collectionTypes.find(
(collectionType) =>
otherCollectionType.canMatch(collectionType) ||
collectionType.accepts(otherCollectionType) ||
otherCollectionType.canMapOver(collectionType),
);
if (connectedCollectionType) {
Expand Down
2 changes: 1 addition & 1 deletion lib/galaxy/model/dataset_collections/matching.py
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ def __attempt_add_to_linked_match(
self.collections[input_name] = hdca
self.subcollection_types[input_name] = subcollection_type
else:
if not self.linked_structure.can_match(structure):
if not self.linked_structure.compatible_shape(structure):
raise exceptions.MessageException(CANNOT_MATCH_ERROR_MESSAGE)
self.collections[input_name] = hdca
self.subcollection_types[input_name] = subcollection_type
Expand Down
6 changes: 4 additions & 2 deletions lib/galaxy/model/dataset_collections/query.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ def direct_match(self, hdca: HdcaLike) -> bool:
collection_type_descriptions = self.collection_type_descriptions
if collection_type_descriptions is not None:
for collection_type_description in collection_type_descriptions:
matches = collection_type_description.can_match_type(hdca.collection.collection_type)
matches = collection_type_description.accepts(hdca.collection.collection_type)
if matches:
return True
return False
Expand All @@ -78,6 +78,8 @@ def can_map_over(self, hdca: HdcaLike):
hdca_collection_type = hdca.collection.collection_type
for collection_type_description in collection_type_descriptions:
# See note about the way this is sorted above.
if collection_type_description.is_subcollection_of_type(hdca_collection_type):
factory = collection_type_description.collection_type_description_factory
hdca_type = factory.for_collection_type(hdca_collection_type)
if hdca_type.can_map_over(collection_type_description):
return collection_type_description
return False
Loading
Loading