classification
Title: PyCapsule_Import fails when name is in the form 'package.module.capsule'
Type: behavior Stage: patch review
Components: Interpreter Core Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: eric.snow, lekma, ncoghlan, petr.viktorin, serhiy.storchaka, skrah
Priority: normal Keywords: patch

Created on 2017-12-23 09:47 by lekma, last changed 2018-05-17 13:34 by ncoghlan.

Pull Requests
URL Status Linked Edit
PR 4988 open python-dev, 2017-12-23 10:15
PR 6898 open serhiy.storchaka, 2018-05-16 10:57
Messages (15)
msg308950 - (view) Author: (lekma) * Date: 2017-12-23 09:47
When capsule name is in the form 'package.module.capsule', PyCapsule_Import fails with an AttributeError (module is never an attribute of package).
msg308990 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-12-24 08:41
PR 4988 will break the case 'module.attr.capsule' if 'module.attr' is not a module.
msg308994 - (view) Author: (lekma) * Date: 2017-12-24 10:10
> PR 4988 will break the case 'module.attr.capsule' if 'module.attr' is
> not a module.
you are right, but is that a case you would expect in an import
statement?
msg315944 - (view) Author: (lekma) * Date: 2018-04-30 09:27
thinking out loud here: maybe both behavior can be exposed, i.e. a PyCapsule_Import that would expect a valid import statement and a Py_GetCapsule that would behave more like getattr?
msg316079 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2018-05-02 15:00
An option would be to use a colon to separate the module(s) from the attribute(s). The "inspect" module CLI does this, for example:
https://docs.python.org/3/library/inspect.html#command-line-interface

$ python3 -m inspect urllib.parse:SplitResult.geturl
    def geturl(self):
        return urlunsplit(self)

A colon can't appear in an identifier, so the old way can be used if there's no colon in the argument, making this backwards compatible.
msg316123 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-05-03 13:33
"package.module:attribute" is also the syntax used in packaging tools to unambiguously separate the name of the module to be imported from the attribute chain to be looked up within that module: https://packaging.python.org/specifications/entry-points/

So +1 from me for extending it to this use case as well.
msg316735 - (view) Author: Eric Snow (eric.snow) * (Python committer) Date: 2018-05-15 21:41
+1 on using ":" as the separator.
msg316794 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-16 11:00
Using ":" makes the syntax irregular. "package.module:attribute", but "module.attribute". And "package.module.attribute" works too if "package.module" already is imported.

It is possible to support importing submodules without using ":".
msg316795 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2018-05-16 11:13
I'm sorry if this interrupts the discussion, but isn't this

https://github.com/plures/xnd/blob/06f6dd82dda9c7bca5df30b121b5cd75c6337609/python/xnd/pyxnd.h#L103

of the form package.module.capsule?

This works, and I just want to make sure it continues to do so.
msg316798 - (view) Author: Stefan Krah (skrah) * (Python committer) Date: 2018-05-16 11:28
Ah, Serhiy stated the reason (module already imported) previously. Sorry for the noise.
msg316799 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-16 11:28
In your case importing xnd automatically imports xnd._xnd and sets it as an attribute of xnd. This case works now. PR 6898 adds support for cases when a submodule is not imported automatically by importing a package.
msg316805 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2018-05-16 13:28
Is "package.module:namespace.attribute" worth supporting? I guess it isn't, at least for now. So Serhyi's patch will work.
msg316915 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-05-17 11:48
The behaviour I'd expect to see:

"module:qual.name" -> "imports module, accesses module.qual.name"
"module.qual.name" -> "no implicit import, accesses module.qual.name"
"package.submodule:qual.name" -> "imports package.submodule, accesses package.submodule.qual.name"
"package.submodule.qual.name" -> "no implicit import, accesses package.submodule.qual.name"

So if you have ":" in the capsule path, you're requesting that the interpreter execute an import with the path up to that point before attempting to resolve the full name reference.

By contrast, if you omit the ":", you're telling the interpreter that you'll take care of ensuring that any required imports have taken place before attempting to resolve the capsule reference.
msg316923 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-05-17 12:42
This change would break virtually all existing usages of PyCapsule_Import(). And it would look very surprising taking into account the name of this function.

Currently "package.submodule.qual.name" imports package and accesses package.submodule.qual.name. You are out of luck if package.submodule was not imported before or if importing package doesn't import package.submodule.
msg316927 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2018-05-17 13:34
Ah, sorry, I misinterpreted Petr's comment, and then didn't look at the details of your PR before commenting.

Having fixed that mistake, I agree that making the naive approach "just work" is a good option.
History
Date User Action Args
2018-05-17 13:34:57ncoghlansetmessages: + msg316927
2018-05-17 12:42:12serhiy.storchakasetmessages: + msg316923
2018-05-17 11:48:03ncoghlansetmessages: + msg316915
versions: + Python 3.8, - Python 3.7
2018-05-16 15:47:53brett.cannonsetnosy: - brett.cannon
2018-05-16 13:28:30petr.viktorinsetmessages: + msg316805
2018-05-16 11:28:53serhiy.storchakasetmessages: + msg316799
2018-05-16 11:28:37skrahsetmessages: + msg316798
2018-05-16 11:13:15skrahsetnosy: + skrah
messages: + msg316795
2018-05-16 11:00:43serhiy.storchakasetmessages: + msg316794
2018-05-16 10:57:26serhiy.storchakasetpull_requests: + pull_request6568
2018-05-15 21:41:27eric.snowsetmessages: + msg316735
2018-05-03 13:33:47ncoghlansetmessages: + msg316123
2018-05-02 15:00:41petr.viktorinsetmessages: + msg316079
2018-05-01 00:50:31brett.cannonsetnosy: + petr.viktorin
2018-04-30 09:27:58lekmasetmessages: + msg315944
2017-12-24 21:09:11serhiy.storchakasetnosy: + brett.cannon, ncoghlan, eric.snow
2017-12-24 10:10:36lekmasetmessages: + msg308994
2017-12-24 08:41:59serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg308990
2017-12-23 10:15:44python-devsetkeywords: + patch
stage: patch review
pull_requests: + pull_request4874
2017-12-23 09:47:09lekmacreate