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

Script decorator breaks type hints #1017

Closed
elliotgunton opened this issue Apr 3, 2024 · 0 comments · Fixed by #1018
Closed

Script decorator breaks type hints #1017

elliotgunton opened this issue Apr 3, 2024 · 0 comments · Fixed by #1018
Labels
type:bug A general bug

Comments

@elliotgunton
Copy link
Collaborator

elliotgunton commented Apr 3, 2024

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

Originally posted by @elliotgunton in #837 (comment)

@elliotgunton elliotgunton added the type:bug A general bug label Apr 3, 2024
elliotgunton added a commit that referenced this issue Apr 15, 2024
* Function signature is now left intact when called elsewhere
* Script decorator still suggests Script kwargs
* Also allows call site typing to use Step/Task type hints

**Pull Request Checklist**
- [x] Fixes #1017 
- [x] [Good commit messages](https://cbea.ms/git-commit/) and/or PR
title

**Description of PR**
See issue for "before" screenshots.

For "after":

Script decorator type hints:

![image](https://github.com/argoproj-labs/hera/assets/17798778/627e1918-b785-453c-a0e6-fa3f3e39617b)

Function call type hints:

![image](https://github.com/argoproj-labs/hera/assets/17798778/56a36c2d-290e-451d-ae3e-0141aea32ef5)


Task/Step type hints (`arguments`, `when`, `name` etc):

![image](https://github.com/argoproj-labs/hera/assets/17798778/f0adead2-eaa4-47c0-b7be-1e71d902f6dc)

---------

Signed-off-by: Elliot Gunton <[email protected]>
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

Successfully merging a pull request may close this issue.

1 participant