-
Notifications
You must be signed in to change notification settings - Fork 2
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
Support deffered types #2
Support deffered types #2
Conversation
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.
Thanks for the PR, I'd make a couple of changes, but everything looks good, I also tested it on my work codebase with around 200 dependencies and it works fine, though we didn't have to use deferred types, mostly because it'll probably mean that you'd have a circular dependency somewhere.
if klass_name := getattr(provider_type, "__name__", None): | ||
self.type_context[klass_name] = provider_type |
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.
I'm not sure, but I think it's safe to assume that all python objects have __name__
attribute? 🤔
if klass_name := getattr(provider_type, "__name__", None): | |
self.type_context[klass_name] = provider_type | |
self.type_context[provider_type.__name__] = provider_type |
I needed that due to |
Co-authored-by: Doctor <[email protected]>
Co-authored-by: Doctor <[email protected]>
I'm not saying you're doing something wrong, you must've had a valid use case for that, also probably with new python versions when |
impl: Any | ||
lifetime: DependencyLifetime | ||
_cached_dependencies: tuple[Dependency[object], ...] | ||
_cached_type: type[_T] # TODO: I think it is redundant. |
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.
What do you think? I think this can be removed. I can find a valid usecase for this
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.
You should provide the type in the provider I think no?
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.
You should provide the type in the provider I think no?
Not necessarily, in most cases type is derived from the object you pass into provider, e.g. Scoped(Test)
would have type Test
I didn't include settings default |
fix #1