Skip to content
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.

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

Already on GitHub? Sign in to your account

Fix callable script linting/typing #837

Open
2 of 4 tasks
elliotgunton opened this issue Nov 3, 2023 · 4 comments
Open
2 of 4 tasks

Fix callable script linting/typing #837

elliotgunton opened this issue Nov 3, 2023 · 4 comments
Labels
type:bug A general bug

Comments

@elliotgunton
Copy link
Collaborator

Pre-bug-report checklist

1. This bug can be reproduced using YAML

2. This bug occurs when...

  • running Hera code without submitting to Argo (e.g. when exporting to YAML)
  • running Hera code and submitting to Argo

Bug report

Describe the bug
A clear and concise description of what the bug is:

When using script functions under e.g. a DAG context, the linter still doesn't know it's actually a Task necessitating casts or # type: ignores. e.g.:

workflow_template.py:15: error: Item "Step" of "Any | Task | Step" has no attribute "on_success"  [union-attr]
workflow_template.py:15: error: Argument 1 to "on_success" of "Task" has incompatible type "Any | Task | Step"; expected "Task"  [arg-type]

To Reproduce
Any Hera examples using Task functions, i.e. on_success or >> operators.

Expected behavior
A clear and concise description of what you expected to happen:
No linting errors.

Environment

  • Version of Hera: 5.9.0

Additional context
Add any other context about the problem here.

Maybe we can fix it with overload functions? Depends what the linter can figure out

@elliotgunton elliotgunton added the type:bug A general bug label Nov 3, 2023
@flaviuvadan
Copy link
Collaborator

It might really be related to python/mypy#1693, in which case I am not sure if this is worth it because the type is determined at run time by the context

@elliotgunton elliotgunton changed the title Fix callable script typing Fix callable script linting/typing Nov 30, 2023
@elliotgunton
Copy link
Collaborator Author

We also saw issues with specifying Task dependencies, e.g.:

workflow_template.py:20: error: Unsupported operand types for >> ("list[Task]" and "Task")  [operator]
workflow_template.py:20: note: Left operand is of type "Task | list[Task]"

@elliotgunton
Copy link
Collaborator Author

Related to this issue is the fact that the script decorator fudged up the type hints for the function. The intention of the _take_annotation_from, seen here

@_take_annotation_from(Script) # type: ignore

was to allow the script decorator itself to set the Script class values, like volumes etc, which does work:

image

But, the unintended side effect was the function itself lost type hints because I thought I needed to add the Script type hints at the call site as well, but actually this should be the type hints for Step/Task, e.g. when doesn't show up:

image

because it's using the Script kwargs for the hints, not TemplateInvocatorSubNodeMixin (i.e. Steps/Tasks) kwargs.

Furthermore, types hints for the function itself broke:

image

@elliotgunton
Copy link
Collaborator Author

Also seeing callable scripts throwing call-arg mypy errors (against a docs example):

python -m mypy main.py
main.py:11: error: Unexpected keyword argument "name"  [call-arg]
main.py:11: error: Unexpected keyword argument "arguments"  [call-arg]
main.py:12: error: Unexpected keyword argument "name"  [call-arg]
main.py:12: error: Unexpected keyword argument "arguments"  [call-arg]
main.py:13: error: Unexpected keyword argument "name"  [call-arg]
main.py:13: error: Unexpected keyword argument "arguments"  [call-arg]
main.py:14: error: Unexpected keyword argument "name"  [call-arg]
main.py:14: error: Unexpected keyword argument "arguments"  [call-arg]
Found 8 errors in 1 file (checked 1 source file)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants