flyteorg / flyte

Scalable and flexible workflow orchestration platform that seamlessly unifies data, ML and analytics stacks.
https://flyte.org
Apache License 2.0
5.37k stars 570 forks source link

[BUG] FlyteFile type parameter causes type checker failures #1300

Open tekumara opened 3 years ago

tekumara commented 3 years ago

FlyteFile uses type parameters to specify metadata about the file type, eg:

return FlyteFile["jpg"](path=out_path)

This causes my type checker (pylance/pyright) to complain because "jpg" is not defined. And when a type parameter isn't supplied in a signature (ie: it's just FlyteFile) then the return type is partially unknown and inferred as FlyteFile[Unknown]

NB: The same applies to FlyteDirectory.

tekumara commented 3 years ago

Some workarounds:

  1. Use a Literal type, eg: return FlyteFile[Literal["jpg"]](path=out_path)
  2. Remove typing.Generic[T] from the FlyteFile type signature, eg: class FlyteFile(os.PathLike):

Both of these keep the use of type parameters. I wonder though if there's a more intuitive or Pythonic way to specify the file format metadata rather than using type parameters. Eg: if it's only needed at runtime, then it could be part of the constructor for a FlyteFile, eg:

return FlyteFile(format="jpg", path=out_path)
kumare3 commented 3 years ago

@tekumara this has been a sticking point for me too.

The problem statement is avoid a cheap/intuitive way for users to provide file formats without needing to define new classes etc. and this has to be in declaration of the task not a implicit return attribute as in the return statement as the Flyte compiler does not introspect the code.

The lint error can be solved if you use JPG=typing.TypeVar("jpg")

And then create FlyteFile[JPG]

But, agree this is weird too.

tekumara commented 3 years ago

@kumare3 what do you think about using Annotated from PEP-593 for this? It seems like it was introduced for this sort of use case.

eg:

@task
def rotate(image_location: str) -> Annotated[FlyteFile, "csv"]:

or even more explicit:

def fields(**kwargs):
    return kwargs

@task
def rotate(image_location: str) -> Annotated[FlyteFile, fields(format="csv")]:

which is basically:

@task
def rotate(image_location: str) -> Annotated[FlyteFile, {"format":"csv"}]:
github-actions[bot] commented 1 year ago

Hello 👋, This issue has been inactive for over 9 months. To help maintain a clean and focused backlog, we'll be marking this issue as stale and will close the issue if we detect no activity in the next 7 days. Thank you for your contribution and understanding! 🙏

github-actions[bot] commented 11 months ago

Hello 👋, This issue has been inactive for over 9 months and hasn't received any updates since it was marked as stale. We'll be closing this issue for now, but if you believe this issue is still relevant, please feel free to reopen it. Thank you for your contribution and understanding! 🙏

github-actions[bot] commented 4 weeks ago

Hello 👋, this issue has been inactive for over 9 months. To help maintain a clean and focused backlog, we'll be marking this issue as stale and will engage on it to decide if it is still applicable. Thank you for your contribution and understanding! 🙏