https://flyte.org logo
Powered by
#ask-the-community
Title
# ask-the-community
r

Rahul Mehta

01/17/2023, 6:27 AM
What’s the minimum python version officially supported for flytekit? Was looking at a mypy improvement to allow the type checker to catch obvious errors in workflows but looks like it’ll need ParamSpec in order to correctly propagate the type annotations in the task/workflow/dynamic decorators. It’s part of Python 3.10, but is also available in typing_extensions for >= py3.7. Is it safe to assume that python 3.7 is the oldest version that flytekit should be guaranteed to work on?
s

Samhita Alla

01/17/2023, 8:57 AM
Flytekit works on python 3.7. But the support for 3.7 ends in ~5 months, and it may not be guaranteed then.
r

Rahul Mehta

01/17/2023, 1:53 PM
Great, this enhancement should work with typing extensions then.
@Eduardo Apolinario (eapolinario) I'd flagged a mypy issue a few weeks ago, it looks like https://peps.python.org/pep-0612/ provides the solution
This may require a larger refactor to how the task/workflow/dynamic decorators work though, but curious on your thoughts here (also cc @Ketan (kumare3) /@Yee)
y

Yee

01/17/2023, 5:25 PM
what sort of refactor?
the inability of those decorators (and launch plans) to supply proper type hints has long been irking me. would be fantastic if you could help fix!
r

Rahul Mehta

01/17/2023, 5:36 PM
The fact that 
@task
& 
@task()
are both valid usages is where the trickiness comes in I believe. If we'd be willing to relax that and enforce 
@task()
as the correct usage, I think this is achievable Also not sure about the best way to wire the `P.args`/`P.kwargs` stuff into 
PythonFunctionTask
(
PythonFunctionTask
itself doesn't even appear to be callable/I think there are some other things that aren't quite right in the typing)
This is the minimal example where I'd like mypy to flag an error:
Copy code
from flytekit.core.task import task
from flytekit.core.workflow import workflow


@task()
def test_task(a: int, b: float) -> float:
    return a + b


@workflow()
def test_wf() -> float:
    return test_task(a=1.0, b="foo")
e

Eduardo Apolinario (eapolinario)

01/17/2023, 6:35 PM
@Rahul Mehta, I started working on this (bear in mind it's very alpha - only handles 
@task
, etc), but it's a start: https://github.com/flyteorg/flytekit/pull/1372/. I double-checked that this is supported in 
typing_extensions
(which also works in python 3.7 as per https://github.com/python/typing_extensions/commit/b697a12f2793655db99dde660bb47458ad36aa55), although I haven't pushed a commit that imports from there instead of 
typing
.
r

Rahul Mehta

01/17/2023, 6:37 PM
interesting..I think you may need to propagate `P.args`/`P.kwargs` for mypy to actually flag the error, was trying this on my own branch this morning and couldn't get mypy to fail w/ the above example
e

Eduardo Apolinario (eapolinario)

01/17/2023, 6:58 PM
with the code as it sits right now in that PR I get this error:
Copy code
❯ mypy workflows/example2.py
workflows/example2.py:5: error: Missing positional argument "_task_function" in call to "task"  [call-arg]
workflows/example2.py:5: error: Argument 1 has incompatible type "Callable[[int, float], float]"; expected <nothing>  [arg-type]
Found 2 errors in 1 file (checked 1 source file)
r

Rahul Mehta

01/17/2023, 6:59 PM
I think that was related to my point above - I think the optional callable for the task function is only due to the fact that the decorator supports @task and @task(). By standardizing on the latter I believe you can avoid the branch
e

Eduardo Apolinario (eapolinario)

01/17/2023, 7:01 PM
got it. This is going to break a lot of people if we go that route though...
r

Rahul Mehta

01/17/2023, 7:01 PM
Yeah, either it’s an api break or we introduce new decorators which are more opinionated
And then slowly phase out the old ones when flytekit has a major version bump?
y

Yee

01/17/2023, 7:02 PM
i don’t think there will be a major version bump for a while fbofw. i think that’s at least a couple years away.
what’s the issue?
task and task() can’t be aligned?
r

Rahul Mehta

01/17/2023, 7:06 PM
Yep, its effectively two different styles of decorators (and ParamSpec supports only the latter IIUC)
We may go ahead and write our own decorators internally in our codebase so we get the typing benefits
y

Yee

01/17/2023, 7:20 PM
oh wait.. we can’t fix just one of them?
r

Rahul Mehta

01/17/2023, 7:43 PM
Could be...I can try sketching out a self-contained example with both styles of decorator & 
ParamSpec
and add that to the discussion on @Eduardo Apolinario (eapolinario)’s PR. We're internally pushing to improve a lot of our typing (and where possible upstream improvements to 3rd-party libs) since it's better for the iteration loop to be able to catch these obvious issues at build time
6 Views
Powered by