-
-
Notifications
You must be signed in to change notification settings - Fork 311
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 propogation through provided #733
Conversation
@rmk135 Any chance that this type fix could be merged as well? |
Thank you for the contribution @philipbjorge . @ZipFile can you please review this PR when you have a moment? |
tests/typing/callable.py
Outdated
attr_getter5: providers.AttributeGetter = provider5.provided.attr | ||
item_getter5: providers.ItemGetter = provider5.provided["item"] | ||
method_caller: providers.MethodCaller = provider5.provided.method.call(123, arg=324) | ||
provided5: Animal = provider5.provided |
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.
This test is not exactly correct. The attribute provider5.provided
returns a provider, not an object of Animal
. I can't recall why it was important, but the current change will introduce a functional change in the typing approach.
To make a correct typing test with the .provided
attribute we would need to do something like that:
provided5: Animal = provider5.provided()
At least, that would be what the library actually does.
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.
@rmk135 updated the tests with .provided()
cc @philipbjorge thanks for flagging
I don't have capacity to address any feedback here and no longer work at the company using this library -- But I've reached out to my previous teammates to see if they can carry the torch :) Thanks for reviewing! Excited to see this project waking back up! |
Co-authored-by: ZipFile <[email protected]>
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.
LGTM
Addresses #731