-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Add Better Typehints using Updated aiosignal #11160
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
base: master
Are you sure you want to change the base?
Conversation
for more information, see https://pre-commit.ci
CodSpeed Performance ReportMerging #11160 will not alter performanceComparing Summary
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #11160 +/- ##
=======================================
Coverage 98.85% 98.85%
=======================================
Files 131 131
Lines 42966 42967 +1
Branches 2314 2314
=======================================
+ Hits 42476 42477 +1
Misses 340 340
Partials 150 150
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
should I try and coming up with a backport if an older version of aiosignal is being utilized? |
What do these changes do?
Adds type hinting to
TraceConfig
using the updated branch of aiosignal. I actually filmed part of this on youtube funny enough and I made sure that I ran pytest tests fortest_tracing.py
passed. This this better than my other pull requests because we no longer need to use wrappers, thank goodness no need for nested code!Are there changes in behavior for the user?
Only 3 which should be the following
_SignalCallback
is now aTypeAlias
as opposed to using aProtocol
styled approach. The Benefit mainly revolves around Aiosignal's NewParamSpec
which seems to be doing more good than I had originally thought. Originally I thought I had to use wrappers but this turns out that this is not the case and rather one of my own VS Extension's fault._T
is now inside of_SignalCallback
so that type checkers can catch unrelated types being carried aroundTraceConfig
callbacks as wrappers optionally.Is it a substantial burden for the maintainers to support this?
Hopefully now I can stop deleting or removing pull requests now that I have figured out the problems on my end and fixed them all. I got rid of the need for needing wrapper properties which is a real blessing. The only thing I could see maintainers maybe raising an Eyebrow for is going to be the
_T
TypeVar being used with_SignalCallback
which I did on purpose to ensure that if TraceConfig has passed a variable such asTraceConfig[dict]
thendict
should be expected by different linters to be passed.Related issue number
Checklist
CONTRIBUTORS.txt
CHANGES/
foldername it
<issue_or_pr_num>.<type>.rst
(e.g.588.bugfix.rst
)if you don't have an issue number, change it to the pull request
number after creating the PR
.bugfix
: A bug fix for something the maintainers deemed animproper undesired behavior that got corrected to match
pre-agreed expectations.
.feature
: A new behavior, public APIs. That sort of stuff..deprecation
: A declaration of future API removals and breakingchanges in behavior.
.breaking
: When something public is removed in a breaking way.Could be deprecated in an earlier release.
.doc
: Notable updates to the documentation structure or buildprocess.
.packaging
: Notes for downstreams about unobvious side effectsand tooling. Changes in the test invocation considerations and
runtime assumptions.
.contrib
: Stuff that affects the contributor experience. e.g.Running tests, building the docs, setting up the development
environment.
.misc
: Changes that are hard to assign to any of the abovecategories.
Make sure to use full sentences with correct case and punctuation,
for example:
Use the past tense or the present tense a non-imperative mood,
referring to what's changed compared to the last released version
of this project.