python-dependency-injector
python-dependency-injector copied to clipboard
ets-labs
allow string imports outside of packages
Problem description
Currently, string imports are not possible outside of packages.
Minimal reproducible example
The following code snippet executed from a script or Jupyter notebook cell raises an AttributeError: 'NoneType' object has no attribute '__package__'
from dependency_injector import providers
deque_factory = providers.Factory('collections.deque')
The trace back reveals that the exception is thrown in src/dependency_injector/providers.pyx in dependency_injector.providers._resolve_calling_package_name().
Proposal for solution
In case the module in dependency_injector.providers._resolve_calling_package_name is None, the package should also be None. The subsequent call to importlib.import_module will handle the situation properly.
IMO this will not be a popular use case, using strings instead of objects is a rather exotic and rare practice..
IMO this will not be a popular use case, using strings instead of objects is a rather exotic and rare practice..
At that time, I was writing an image processing application, where the user could provide a list of filters, e.g. something like a Gaussian filter with given sigma and window size, within a config file. However, I got the feature request if I could also allow filters from other libraries, e.g. scipy, skimage and openCV... or a custom filter... So I thought, let the user select whatever they want by providing the import string and the configuration inside the config file. Then I saw that the factory provider can handle string imports, https://python-dependency-injector.ets-labs.org/providers/factory.html#string-imports. So I thought "No problem!"... However, my first attempt resulted in the above error.
Generally, I agree that this is an exotic and rare practice. Looking at it today reviewing my own PR, I would even argue that providing a string import to another library is unsafe, so it would require either a safe alternative like the safe_load in yaml, or an explicitly unsafe factory provider that allows this...
IMHO, you may abandon the string imports from the factory provider and add an explicit string import provider or delegate the import to the users providing some examples in the documentation. This may shrink the interface to a single way of doing things. However, I can just imagine the difficulties of maintaining such a popular library and dealing with breaking changes. BTW, thanks for providing and maintaining this library!
I could implement the string import provider that allows unsafe imports if you agree.
What do you think?
IMO this will not be a popular use case, using strings instead of objects is a rather exotic and rare practice..
At that time, I was writing an image processing application, where the user could provide a list of filters, e.g. something like a Gaussian filter with given sigma and window size, within a config file. However, I got the feature request if I could also allow filters from other libraries, e.g. scipy, skimage and openCV... or a custom filter... So I thought, let the user select whatever they want by providing the import string and the configuration inside the config file. Then I saw that the factory provider can handle string imports, https://python-dependency-injector.ets-labs.org/providers/factory.html#string-imports. So I thought "No problem!"... However, my first attempt resulted in the above error.
Generally, I agree that this is an exotic and rare practice. Looking at it today reviewing my own PR, I would even argue that providing a string import to another library is unsafe, so it would require either a safe alternative like the
safe_loadin yaml, or an explicitly unsafe factory provider that allows this...IMHO, you may abandon the string imports from the factory provider and add an explicit string import provider or delegate the import to the users providing some examples in the documentation. This may shrink the interface to a single way of doing things. However, I can just imagine the difficulties of maintaining such a popular library and dealing with breaking changes. BTW, thanks for providing and maintaining this library!
I could implement the string import provider that allows unsafe imports if you agree.
What do you think?
I'm just a user of this package, but I made my own analogue of the injector and understand how much it works inside. User experience is important, and I think that the provider should also be specified explicitly, and not as a string. And explicit is better than implicit!
but if we refer to specific arguments, I have two arguments in favor of not working with strings:
- less maintainable code (the best code is the one that is not written, especially in this specific case);
- imagine that you want to use more than one DI container. And in this case you will not get by with a string, because you will need to specify a specific container and its provider:
class Container1(DeclarativeContainer): ...
class Container2(DeclarativeContainer): ...
@inject
def do_smth(dep = Provide["provider"]): ...
# string definitely won’t help here, because there is more than one container
[...] I think that the provider should also be specified explicitly, and not as a string. And [...]
but if we refer to specific arguments, I have two arguments in favor of not working with strings:
- less maintainable code (the best code is the one that is not written, especially in this specific case);
- imagine that you want to use more than one DI container. And in this case you will not get by with a string, because you will need to specify a specific container and its provider:
class Container1(DeclarativeContainer): ... class Container2(DeclarativeContainer): ... @inject def do_smth(dep = Provide["provider"]): ... # string definitely won’t help here, because there is more than one container
Even though they are related, I think we are talking about different string imports. While I am talking about the first argument of providers.Factory, see https://python-dependency-injector.ets-labs.org/providers/factory.html#string-imports, you are talking about string identifiers during wiring, see https://python-dependency-injector.ets-labs.org/wiring.html#string-identifiers.
Please note that I am not generally against string imports, see my use-case above. One motivation for the string identifiers feature is given in the documentation: "You could also use string identifiers to avoid a dependency on a container:". However, this is out of scope regarding this PR. The motivation for the string imports in factory providers is not explicitly given, but I can imagine the case where you want to avoid expensive imports that are not used in every instance of your application, e.g. various supported back-ends which live in different sub-modules where you select only one.
An addition for the feature of this PR would be an unsafe attribute for the providers.Factory which would allow the imports from modules outside of the package. The default would be False and it should be possible to set it per instance. I will think further about it and update this PR.
I just realized, that the above error message is specific to working in an interactive mode, e.g. Jupyter notebooks.