You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Using __name__ on a non-package module results in "not a package" error #60
But in importlib_resources, this approach doesn't work.
>>> importlib_resources.read_binary(queso.validator.tests.test_options.__name__, 'colliding questions.json')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/jaraco/Dropbox/code/yg/G/queso/.tox/py36/lib/python3.6/site-packages/importlib_resources/_py3.py", line 142, in read_binary
package = _get_package(package)
File "/Users/jaraco/Dropbox/code/yg/G/queso/.tox/py36/lib/python3.6/site-packages/importlib_resources/_py3.py", line 40, in _get_package
raise TypeError('{!r} is not a package'.format(package))
TypeError: 'queso.validator.tests.test_options' is not a package
In other words, importlib_resources seems to be more stringent about the input, requiring the parameter to be a package and not simply a module.
Is this limitation intentional? If so, the migration docs should include a note about this incompatibility.
Is there something weird about queso.validator.tests.test_options? Can you reproduce this with something in the Python stdlib? This works for me:
Python 3.6.5+ (heads/3.6:b0ca398cab, Apr 24 2018, 18:02:36)
[GCC 4.2.1 Compatible Apple LLVM 9.1.0 (clang-902.0.39.1)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import test.support
>>> test.support.__spec__.submodule_search_locations
['/usr/local/lib/python3.6/test/support']
>>> test.support.__name__
'test.support'
>>> from importlib_resources import *
>>> x = read_binary(test.support.__name__, '__init__.py')
>>>
Is queso.validator.tests.test_options a non-package module? In that case, yes it's expected behavior:
>>> import importlib_resources._py3
>>> x = read_binary(importlib_resources._py3.__name__, '__init__.py')
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "/Users/barry/projects/implib/resources/importlib_resources/_py3.py", line 142, in read_binary
package = _get_package(package)
File "/Users/barry/projects/implib/resources/importlib_resources/_py3.py", line 40, in _get_package
raise TypeError('{!r} is not a package'.format(package))
TypeError: 'importlib_resources._py3' is not a package
>>>
Is pkg_resources using the parent package containing the module? This would be the case regardless of whether you're passing in the module name via __name__ or the module object.
Getting back to this, I guess pkg_resources finds the parent package of the non-package module? Is this behavior we want to preserve? I'd rather not, but if this use case is pervasive then maybe we should. @brettcannon what do you think?
Yes, queso.validator.tests.test_options is a non-package module (it's a module in a package). pkg_resourcesdoes resolve this (by way of dirname(__file__)).
I suggest that since __name__ can't be used in an arbitrary module (to find resources in the package containing that module), this project should instead recommend use of __package__, but should provide a compatibility shim for Python 2, such that one could write:
Curiously, when I test on Python 2 now, I see that __package__ is available for queso.validator.tests.test_options, so it seems that maybe recent versions of Python 2.7 added support for this property?
Indeed, just going back to Python 2.7.12, I see __package__ is defined, but None:
jaraco@dev-jaraco:~$ mkdir foo
jaraco@dev-jaraco:~$ touch foo/__init__.py
jaraco@dev-jaraco:~$ touch foo/bar.py
jaraco@dev-jaraco:~$ python2.7
Python 2.7.12 (default, Nov 20 2017, 18:23:56)
[GCC 5.4.0 20160609] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import foo.bar
>>> foo.bar.__package__
>>>
So technically, in Python 3 this is as simple as using __spec__.parent as that is the same as __spec__.name for packages and is set to the containing package for modules (it's "" for top-level modules).
As for API change to support this, I'm -0. It's a small nicety, but I would rather people be explicit with what they are anchoring off of rather than calculate it implicitly because someone happened to pass in a submodule. It also alleviates the very minor worry of someone who converted a package to a module and then gets bit by that shift.
Perhaps what you're suggesting is that loading a resource in one's package should not be allowed unless the developer explicitly type the package name. Obviously that's possible, but it does mean that only Python 3 users get the nice experience. If __spec__.parent works on Python 3, it sure would be nice if there was a helper to resolve a comparable value on Python 2.
I still don't see why a helper function couldn't provide a compatibility shim:
defpackage_for(globals):
""" Given a module's globals, return its package. """try:
returnglobals['__spec__'].parentexceptKeyError:
passis_module=inspect.ismodule(__import__(globals['__name__']))
returnglobals['__name__'] ifis_moduleelseglobals['__name__'].rpartition('.')[0]
Or at least provide the Python 2 logic for resolving a package from a module, so one could at least write:
Note, I'm not suggesting changing the API for importlib_resources.read_binary (or its comparable functions), only to provide a means to resolve a package from a module.
It seems unnecessarily constraining and brittle to require a module to name its package by name (and then require that to be kept in sync). It seems comparable to only allowing absolute imports and never relative imports.
IMO, the resource API should provide for resources relative to a module and not leave it to each module to determine/declare in which package it is contained.
If you're not convinced, I'll drop it and focus on the documentation that clarifies this pitfall.
Why wouldn't it just be importlib_resources.read_binary(module.__package__, filename) in both Python 2 and 3? E.g.
Python 2.7.15 (default, Jun 17 2018, 12:46:58)
[GCC 4.2.1 Compatible Apple LLVM 9.1.0 (clang-902.0.39.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from importlib_resources import read_text
>>> import email.utils
>>> source = read_text(email.utils.__package__, 'utils.py')
>>> len(source)
10026
ython 3.6.6+ (heads/3.6:b1707abd74, Jun 30 2018, 13:04:54)
[GCC 4.2.1 Compatible Apple LLVM 9.1.0 (clang-902.0.39.2)] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> from importlib_resources import read_text
>>> import email.utils
>>> source = read_text(email.utils.__package__, 'utils.py')
>>> len(source)
13897
WTF? @brettcannon just tried it with a bespoke package layout and you're right, it doesn't work for 2.7.15 in that case. So why does it work for email.utils?
Aside from that, here's the philosophical question. If we add support for this as a nod to the mission of this library to replace pkg_resources (i.e. having this will make it easier to port away from the latter), then it seems more reasonable to support #58 as well, since that's a pain point for porters too. I've said over there that I'm conditionally in favor of that if we get a reasonable looking contribution.
So, Brett and I muse that if you were by chance to submit a PR for that and it looked okay, we'd be cool with accepting this one too. But in that case, I think allowing modules (objects and names) in the package argument would be better than a convenience function since it's a more direct compatibility compromise.
We'd need to update the documentation too, but I don't think we'd need to change the type signatures. This and #58 would require a major version bump IMO, and I would be disinclined to backport these changes to Python 3.7.
As I think about this more, I'm leaning toward not proposing a change. I'm particularly reluctant to make the interface(s) less precise, especially since the expanded interface would encourage the anti-pattern (using __name__ instead of __package__). I think the alternative (requiring an explicit provision of the package name) is preferable for packages that need to support older Pythons (prior to 2.7.15).
Activity
jaraco commentedon Oct 21, 2020
In GitLab by @jaraco on Jun 3, 2018, 13:53
I note that
__package__works on later Pythons, but not on Python 2.7.jaraco commentedon Oct 21, 2020
In GitLab by @warsaw on Jun 4, 2018, 21:00
Is there something weird about
queso.validator.tests.test_options? Can you reproduce this with something in the Python stdlib? This works for me:Is
queso.validator.tests.test_optionsa non-package module? In that case, yes it's expected behavior:Is
pkg_resourcesusing the parent package containing the module? This would be the case regardless of whether you're passing in the module name via__name__or the module object.jaraco commentedon Oct 21, 2020
In GitLab by @warsaw on Jun 4, 2018, 21:01
assigned to @warsaw
jaraco commentedon Oct 21, 2020
In GitLab by @warsaw on Sep 7, 2018, 19:20
Getting back to this, I guess
pkg_resourcesfinds the parent package of the non-package module? Is this behavior we want to preserve? I'd rather not, but if this use case is pervasive then maybe we should. @brettcannon what do you think?jaraco commentedon Oct 21, 2020
In GitLab by @jaraco on Sep 11, 2018, 09:03
Yes,
queso.validator.tests.test_optionsis a non-package module (it's a module in a package).pkg_resourcesdoes resolve this (by way ofdirname(__file__)).I suggest that since
__name__can't be used in an arbitrary module (to find resources in the package containing that module), this project should instead recommend use of__package__, but should provide a compatibility shim for Python 2, such that one could write:(or similar) to resolve the package of a module.
Curiously, when I test on Python 2 now, I see that
__package__is available forqueso.validator.tests.test_options, so it seems that maybe recent versions of Python 2.7 added support for this property?Indeed, just going back to Python 2.7.12, I see
__package__is defined, butNone:jaraco commentedon Oct 21, 2020
In GitLab by @brettcannon on Sep 11, 2018, 18:21
So technically, in Python 3 this is as simple as using
__spec__.parentas that is the same as__spec__.namefor packages and is set to the containing package for modules (it's""for top-level modules).As for API change to support this, I'm -0. It's a small nicety, but I would rather people be explicit with what they are anchoring off of rather than calculate it implicitly because someone happened to pass in a submodule. It also alleviates the very minor worry of someone who converted a package to a module and then gets bit by that shift.
jaraco commentedon Oct 21, 2020
In GitLab by @jaraco on Sep 11, 2018, 22:13
I'm struggling to devise a straightforward way to replace the pkg_resources code for this use-case.
So the recommendation is for a module that wishes to load resources from the package containing that module that they write:
That seems somehow less preferable to the code this intends to replace:
Perhaps what you're suggesting is that loading a resource in one's package should not be allowed unless the developer explicitly type the package name. Obviously that's possible, but it does mean that only Python 3 users get the nice experience. If
__spec__.parentworks on Python 3, it sure would be nice if there was a helper to resolve a comparable value on Python 2.I still don't see why a helper function couldn't provide a compatibility shim:
Or at least provide the Python 2 logic for resolving a package from a module, so one could at least write:
Note, I'm not suggesting changing the API for importlib_resources.read_binary (or its comparable functions), only to provide a means to resolve a package from a module.
It seems unnecessarily constraining and brittle to require a module to name its package by name (and then require that to be kept in sync). It seems comparable to only allowing absolute imports and never relative imports.
IMO, the resource API should provide for resources relative to a module and not leave it to each module to determine/declare in which package it is contained.
If you're not convinced, I'll drop it and focus on the documentation that clarifies this pitfall.
jaraco commentedon Oct 21, 2020
In GitLab by @warsaw on Sep 12, 2018, 13:02
Why wouldn't it just be
importlib_resources.read_binary(module.__package__, filename)in both Python 2 and 3? E.g.jaraco commentedon Oct 21, 2020
In GitLab by @jaraco on Sep 12, 2018, 13:03
See above:
jaraco commentedon Oct 21, 2020
In GitLab by @warsaw on Sep 12, 2018, 13:26
WTF? @brettcannon just tried it with a bespoke package layout and you're right, it doesn't work for 2.7.15 in that case. So why does it work for
email.utils?Aside from that, here's the philosophical question. If we add support for this as a nod to the mission of this library to replace
pkg_resources(i.e. having this will make it easier to port away from the latter), then it seems more reasonable to support #58 as well, since that's a pain point for porters too. I've said over there that I'm conditionally in favor of that if we get a reasonable looking contribution.So, Brett and I muse that if you were by chance to submit a PR for that and it looked okay, we'd be cool with accepting this one too. But in that case, I think allowing modules (objects and names) in the
packageargument would be better than a convenience function since it's a more direct compatibility compromise.We'd need to update the documentation too, but I don't think we'd need to change the type signatures. This and #58 would require a major version bump IMO, and I would be disinclined to backport these changes to Python 3.7.
jaraco commentedon Oct 21, 2020
In GitLab by @jaraco on Oct 31, 2018, 15:02
As I think about this more, I'm leaning toward not proposing a change. I'm particularly reluctant to make the interface(s) less precise, especially since the expanded interface would encourage the anti-pattern (using
__name__instead of__package__). I think the alternative (requiring an explicit provision of the package name) is preferable for packages that need to support older Pythons (prior to 2.7.15).jaraco commentedon Oct 21, 2020
In GitLab by @jaraco on Oct 31, 2018, 15:02
closed
7 remaining items
VALI-9484 Update pint to latest version (#20)
binman: migrate from pkg_resources to importlib
binman: migrate from pkg_resources to importlib
binman: migrate from pkg_resources to importlib
binman: migrate from pkg_resources to importlib
binman: migrate from pkg_resources to importlib
binman: migrate from pkg_resources to importlib
binman: migrate from pkg_resources to importlib
binman: migrate from pkg_resources to importlib
binman: migrate from pkg_resources to importlib
binman: migrate from pkg_resources to importlib
binman: migrate from pkg_resources to importlib
BACKPORT: binman: migrate from pkg_resources to importlib
BACKPORT: binman: migrate from pkg_resources to importlib
binman: migrate from pkg_resources to importlib