Skip to content

ENH: allow typing.Union in action outputs #754

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
misialq wants to merge 1 commit into from
Closed

ENH: allow typing.Union in action outputs #754

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 20 additions & 0 deletions qiime2/core/type/signature.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
import copy
import itertools
import tempfile
from typing import get_origin, Union, get_args

import qiime2.sdk
import qiime2.core.type as qtype
Expand Down Expand Up @@ -484,6 +485,25 @@ def coerce_given_outputs(self, output_views, output_types, scope,
name=name, key=key, idx=idx, size=size)
output[key] = self._create_output_artifact(
provenance, collection_name, scope, spec, view)
elif isinstance(get_origin(spec.view_type), type(Union)):
union_resolved = False
for arg in get_args(spec.view_type):
if isinstance(output_view, arg):
# we need a new spec with the correct view type
spec = ParameterSpec(
qiime_type=spec.qiime_type, view_type=arg,

@ebolyen ebolyen Mar 4, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having not looked very much into it, should this be something we handle in transform.py? It's a scary place, but it seems like where I would expect to deconstruct the union.

@misialq misialq Mar 5, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ebolyen, I did have a look but tbh I wasn't sure how exactly to do it there... If you have any hints, let me know 馃檹

@lizgehret lizgehret Aug 8, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @ebolyen, resurfacing this thread - any thoughts on the above?

default=spec.default, description=spec.description
)
output = self._create_output_artifact(
provenance, name, scope, spec, output_view
)
union_resolved = True
break
if not union_resolved:
raise TypeError(
"Expected output view type to be one of %r, received %r" %
([x.__name__ for x in get_args(spec.view_type)],
type(output_view).__name__))
elif type(output_view) is not spec.view_type:
raise TypeError(
"Expected output view type %r, received %r" %
Expand Down