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 type annotations on template decorators #1211

Merged

Conversation

alicederyn
Copy link
Collaborator

@alicederyn alicederyn commented Sep 25, 2024

Pull Request Checklist

Description of PR
Currently, mypy raises a misc error when using a decorator from TemplateDecoratorFuncsMixin, complaining that the method "does not accept self argument".

This PR uses Concatenate to add the missing parameter.

@alicederyn alicederyn added semver:patch A change requiring a patch version bump type:bug A general bug python Dependabot pull requests that update Python dependencies labels Sep 25, 2024
Use `Concatenate` to add the missing `self` parameter to the class methods
annotated with `@_add_type_hints` in `TemplateDecoratorFuncsMixin`. Without
this, mypy raises a `misc` error on usage, complaining that the method "does
not accept self argument".

Signed-off-by: Alice Purcell <alicederyn@gmail.com>
@alicederyn alicederyn force-pushed the fix.template.decorator.annotation branch from 02f7c4e to 884a6cc Compare September 25, 2024 19:26
@@ -474,7 +475,10 @@ def _add_type_hints(
) -> Callable[
...,
Callable[
PydanticKwargs, # this adds type hints to the underlying *library* function kwargs
Concatenate[
object, # this is the `self` parameter
Copy link
Collaborator Author

@alicederyn alicederyn Sep 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to capture the correct class type from the decorated function, but couldn't find a way to do it that didn't break both mypy and pyright. I verified that both mypy and pyright are happy with object. (Any also seems to work if that's preferred.)

Copy link

codecov bot commented Sep 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 86.0%. Comparing base (d553546) to head (884a6cc).
Report is 201 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##            main   #1211     +/-   ##
=======================================
+ Coverage   81.7%   86.0%   +4.2%     
=======================================
  Files         54      60      +6     
  Lines       4208    4040    -168     
  Branches     889     840     -49     
=======================================
+ Hits        3439    3475     +36     
+ Misses       574     392    -182     
+ Partials     195     173     -22     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Collaborator

@jeongukjae jeongukjae left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@elliotgunton elliotgunton merged commit 005db40 into argoproj-labs:main Sep 27, 2024
22 checks passed
@alicederyn alicederyn deleted the fix.template.decorator.annotation branch September 27, 2024 10:02
@elliotgunton elliotgunton added type:task A general task and removed type:bug A general bug python Dependabot pull requests that update Python dependencies labels Sep 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver:patch A change requiring a patch version bump type:task A general task
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Mypy misc error using new Workflow/TemplateSet decorators
3 participants