feat: Support Optional samplesheet parameters

[msto]

Workflow parameters which are mapped to rows in the Latch registry must be declared as a list of dataclasses in the workflow function.

When such a parameter is part of a ForkBranch (e.g. when maintaining backwards compatibility for a workflow which previously supported input of a manually generated samplesheet), the parameter does not receive a value from Flyte and must have a default value defined in the workflow function signature. It would be preferable to set this default value to None rather than an empty list, to avoid providing a mutable argument default.

It would be helpful if the workflow parameter could be made Optional to support this use case.

This PR adds support for Optional samplesheet parameters.

[msto]

Unit tests pass locally for me under both 3.9 and 3.10, and mypy identifies no issues on the modified lines.

[ayushkamat]

Thanks for the contribution! I appreciate that you took the time to look through PEP 585 and PEP 604 and provide some clarification there.

Overall this is much more complexity than necessary, given that we will likely not be reusing this code for anything else and that what is written is highly specified to this problem.

Heres how I would do it:

if sys.version_info >= (3, 10):
    from types import UnionType
else:

    class UnionType: ...

def is_valid_samplesheet_param(typ: Type) -> bool:
    origin = get_origin(typ)
    if origin is None:
        return False

    args = get_args(typ)

    if origin is Union or issubclass(origin, UnionType):
        if len(args) != 2:
            return False

        for arg in args:
            if arg is type(None) or is_valid_samplesheet_param(arg):
                continue

            return False

        return True

    return issubclass(origin, list) and len(args) == 1 and is_dataclass(args[0])

[ayushkamat]

this is the wrong place for this check, i cant remember off the top of my head if we throw a semantic error already for not having a type annotation but we should do this outside of a samplesheet specific check (itll always be an error regardless of whether or not the specific param is a samplesheet)

[msto]

Given that the project does not enforce any type checking, I would prefer to include runtime checks on all argument types.

[msto]

(I'm happy to make this raise a TypeError instead, though in context I thought returning False was sensible.)

[ayushkamat]

del

[msto]

Some form of this is required.

The call to issubclass on the next line will raise a TypeError if it is passed an object that isn't a class, which typing.Union is not.

i.e. the following raises an error:

from typing import Optional, get_origin
origin = get_origin(Optional[T])

issubclass(origin)

I've replaced with a requirement that origin is a class.

[nh13]

This is super helpful, thank-you @msto. @ayushkamat what do we need to do to get this merged and released?