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

Proper type hints for @metric_scope decorator to work nicely with mypy #70

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

pierresouchay
Copy link

When using @metric_scope on a typed function, mypy produces this kind of error:

error: Untyped decorator makes function "handler" untyped

This fix avoid this error and will let people using typehint now having those kind
of error messages.

See https://mypy.readthedocs.io/en/stable/generics.html#declaring-decorators

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@pierresouchay
Copy link
Author

Hello @jaredcnance,

By chance, could anyone review it? (It is a very small change)

Thanks for your work

Regards

@pierresouchay
Copy link
Author

Hello @jaredcnance, @02strich, @ridha ?

The PR is quite simple, anyone for a review?

Kind Regards

@pierresouchay
Copy link
Author

Ping @jaredcnance

When using @metric_scope on a typed function, mypy produces this kind of error:

  error: Untyped decorator makes function "handler" untyped

This fix avoid this error and will let people using typehint now having those kind
of error messages.

See https://mypy.readthedocs.io/en/stable/generics.html#declaring-decorators
@pierresouchay pierresouchay force-pushed the proper_type_hints_to_work_nicely_with_mypy branch from 48e0477 to d147a36 Compare June 7, 2022 14:16
@pierresouchay
Copy link
Author

@jaredcnance Do you think you might have a look?

@willbeaufoy
Copy link

Merging this would be useful for me.

@jedkass
Copy link

jedkass commented Apr 29, 2024

@jaredcnance @markkuhn @ramprasadg Any chance any of y'all might have a few mins to review this small PR? It looks like it just needs one more approval

@pierresouchay
Copy link
Author

@jaredcnance @markkuhn @ramprasadg Any chance any of y'all might have a few mins to review this small PR? It looks like it just needs one more approval

3y after... I am not optimistic ;)

@jedkass
Copy link

jedkass commented May 1, 2024

@pierresouchay Haha idk after that @ramprasadg approval, I'm feeling pretty good about it! :D (@ramprasadg thank you for taking a look!! 🙏).

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

Successfully merging this pull request may close these issues.

5 participants