| msg255838 - (view) |
Author: Brett Cannon (brett.cannon) *  |
Date: 2015-12-03 20:48 |
| When you do a relative import, __package__ is used to help resolve it, but if it isn't defined we fall back on using __name__ and __path__. We should probably consider raising an ImportWarning if __package__ isn't defined so that some day we can consider cleaning up __import__ and its call signature to be a bit more sane and drop the `globals` argument. It would also help people catch errors where they went overboard deleting attributes off a module. We should probably even extend it to start using __spec__.parent and then falling back to __package__, and only after both are found missing do we raise the ImportWarning about needing to use __name__ and __path__. That way __import__ can simply start taking in the spec of the calling module to do all of its work instead of having to pass all of globals(). |
|
|
| msg258171 - (view) |
Author: Rose Ames (superluser) * |
Date: 2016-01-13 21:34 |
| Patch with tests. Not sure if importlib.h should be included? |
|
|
| msg258172 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2016-01-13 21:36 |
| Including importlib.h doesn't hurt. |
|
|
| msg258173 - (view) |
Author: Rose Ames (superluser) * |
Date: 2016-01-13 21:39 |
| that's what I figured. |
|
|
| msg258178 - (view) |
Author: Rose Ames (superluser) * |
Date: 2016-01-13 23:00 |
| Thanks for the quick review, new patch uploaded. |
|
|
| msg258329 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2016-01-15 21:33 |
| New changeset 6908b2c9a404 by Brett Cannon in branch 'default': Issue #25791: Raise an ImportWarning when __spec__ or __package__ are https://hg.python.org/cpython/rev/6908b2c9a404 |
|
|
| msg258330 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2016-01-15 21:33 |
| Thanks for the patch, Rose! I did notice your review comments about the missing goto and the stacklevel and I simply added them myself. |
|
|
| msg258354 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2016-01-16 04:43 |
| Favouring __spec__.parent over __package__ will break the documented workaround in PEP 366 for enabling explicit relative imports from __main__ even when a module is run directly instead of via -m: if __name__ == "__main__" and __package__ is None: __package__ = "expected.package.name" It may be that workaround is something we *want* to break (as per http://bugs.python.org/issue21762#msg258332 ), but if so, it's at least worthy of a porting note in the What's New document. |
|
|
| msg258400 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2016-01-16 18:18 |
| As I commented on another issue, I think I'm going to tweak the change to continue to prefer __package__, but raise ImportWarning when it doesn't match __spec__.parent. Once we do that for all attributes we can wait until Python 2.7 is done and then swap priority to __spec__ and raise a DeprecationWarning if __spec__ is missing. That way post-Python 2.7 we can move entirely to a spec-based import system. |
|
|
| msg258842 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2016-01-22 23:27 |
| New changeset 219c44fe8968 by Brett Cannon in branch 'default': Issue #25791: Warn when __package__ != __spec__.parent. https://hg.python.org/cpython/rev/219c44fe8968 |
|
|
| msg294632 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2017-05-28 08:32 |
| This new warning is introducing difficulties for some Cython-compiled modules: https://github.com/cython/cython/issues/1720 |
|
|
| msg294652 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2017-05-28 18:03 |
| Is Cython not defining actual module objects or working around types.ModuleType? I'm just trying to figure out how a Cython module is ending up in a place where the attributes that are set by PyModule_NewObject() aren't there (https://github.com/python/cpython/blob/ac5bbd43bc7b769c13ae0412cb28a3521f4d4ff1/Objects/moduleobject.c#L80). |
|
|
| msg294653 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2017-05-28 18:10 |
| AFAICT, Cython simply calls PyModule_Create() on Python 3. |
|
|
| msg294656 - (view) |
Author: Brett Cannon (brett.cannon) * |
Date: 2017-05-28 19:18 |
| OK, so the warning is triggered if __package is None or __spec__ is None (https://github.com/python/cpython/blob/ac5bbd43bc7b769c13ae0412cb28a3521f4d4ff1/Lib/importlib/_bootstrap.py#L1038). That's defined in _calc___package__() which is only called if index != 0 when calling __import__() (https://github.com/python/cpython/blob/ac5bbd43bc7b769c13ae0412cb28a3521f4d4ff1/Lib/importlib/_bootstrap.py#L1062 or https://github.com/python/cpython/blob/ac5bbd43bc7b769c13ae0412cb28a3521f4d4ff1/Python/import.c#L1520). So the question becomes how is Cython importing modules? This warning should only be triggered if you're attempting a relative import from within a package (and thus have a level > 0). Based on Antoine's Cython issue, since something like multidict isn't a package it would suggest either Cython is setting the level > 0 when it doesn't mean to or there's a bug somewhere with level == 0 and yet Python is still trying to calculate the parent package for no reason. |
|
|
| msg294657 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2017-05-28 19:25 |
| multidict is a package, the Cython module is multidict._multidict. Cython translated "import sys" into this: __pyx_t_1 = __Pyx_Import(__pyx_n_s_sys, 0, -1); if (unlikely(!__pyx_t_1)) __PYX_ERR(0, 5, __pyx_L1_error) And __Pyx_Import is this: https://gist.github.com/pitrou/9a0d94aba3495ce3eb8630d4797d67bb Note that __Pyx_MODULE_NAME is defined thusly: #define __Pyx_MODULE_NAME "multidict._multidict" So perhaps Cython is indeed attempting a PyImport_ImportModuleLevelObject() call with a level of 1 for "import sys" before falling back on a level of -1... Why I don't know. (also since __Pyx_MODULE_NAME seems known, Cython could actually define __package__) |
|
|
| msg294668 - (view) |
Author: Alyssa Coghlan (ncoghlan) * |
Date: 2017-05-29 04:10 |
| Is it possible Cython is still supporting pre-PEP-328 style implicit relative imports, even in Python 2.7+? |
|
|
| msg294677 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2017-05-29 07:10 |
| > Is it possible Cython is still supporting pre-PEP-328 style implicit relative imports, even in Python 2.7+? That might be the case. I can't find anything in the Cython docs. Stefan, could you shed a light? |
|
|
| msg294678 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2017-05-29 07:14 |
| Ok, I did an experiment. I added "from __future__ import absolute_import" at the top of _multidict.c, and after a recompile the warning went away. What changed was that the following line: __pyx_t_1 = __Pyx_Import(__pyx_n_s_sys, 0, -1); if (unlikely(!__pyx_t_1)) __PYX_ERR(0, 5, __pyx_L1_error) was replaced with: __pyx_t_1 = __Pyx_Import(__pyx_n_s_sys, 0, 0); if (unlikely(!__pyx_t_1)) __PYX_ERR(0, 3, __pyx_L1_error) Ignore the change from "3" to "5", which is the line number for the traceback. The relevant change is the "level" argument to __Pyx_Import() which went from -1 to 0. |
|
|
| msg296951 - (view) |
Author: Stefan Behnel (scoder) * |
Date: 2017-06-26 21:50 |
| Sorry for not responding, missed the message, it seems. Cython has to support old-style relative imports also in Py3 because that's how the user code was originally written, using Py2-style syntax and semantics. Most Cython code has not been converted to Py3 syntax/semantics, but still needs to work in Py3. From a user perspective, it's best to switch on "__future__.absolute_import" and use explicit relative imports (or write Py3 code), because it reduces the overhead at import time. I'm hoping to look into multi-step module initialisation this summer. As Nick noted, this might also help with this issue. |
|
|