-
Notifications
You must be signed in to change notification settings - Fork 403
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(metrics): explicit type to single_metric ctx manager #865
fix(metrics): explicit type to single_metric ctx manager #865
Conversation
Codecov Report
@@ Coverage Diff @@
## develop #865 +/- ##
=========================================
Coverage 99.90% 99.90%
=========================================
Files 118 118
Lines 5125 5126 +1
Branches 283 571 +288
=========================================
+ Hits 5120 5121 +1
Misses 2 2
Partials 3 3
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor change to use Generator[SingleMetric, None, None]
due to a new issue discovered in Python typeshed when using Iterator[]
in this context.
I created a new project using VSCode and PyLance and I couldn't reproduce this issue, same with PyCharm so perhaps their InteliSense is smart by inspecting the yield type Nevertheless, TIL why we can't use For the sake of correctness regardless of what IDE one might use, I'm gonna commit the Generator suggestion to avoid issues and merge it. |
…tools-python into develop * 'develop' of https://github.com/awslabs/aws-lambda-powertools-python: fix(metrics): explicit type to single_metric ctx manager (#865)
Interesting. I poured over a ton of python code and standard libs and Iterator seemed like the best fit. I will read up on this. |
Yep same. It seems to be a “new” thing
…On Wed, 8 Dec 2021 at 16:15, Shane R. Spencer ***@***.***> wrote:
Interesting. I poured over a ton of python code and standard libs and
Iterator seemed like the best fit. I will read up on this.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub
<#865 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAZPQBAG6AJVV5US52MDV5LUP5ZA3ANCNFSM5JH6AMOA>
.
|
Issue #, if available:
Description of changes:
Simply update type return for
metrics.metric.single_metric
.Checklist
Breaking change checklist
RFC issue #:
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.