[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Stefan Behnel]

Stefan Behnel added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Antoine Pitrou]

Antoine Pitrou added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Antoine Pitrou]

Antoine Pitrou added the comment:

> 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?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Nick Coghlan]

Nick Coghlan added the comment:

Is it possible Cython is still supporting pre-PEP-328 style implicit relative 
imports, even in Python 2.7+?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Antoine Pitrou]

Antoine Pitrou added the comment:

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__)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Brett Cannon]

Brett Cannon added the comment:

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.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Antoine Pitrou]

Antoine Pitrou added the comment:

AFAICT, Cython simply calls PyModule_Create() on Python 3.

--
nosy: +scoder

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Brett Cannon]

Brett Cannon added the comment:

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).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Antoine Pitrou]

Antoine Pitrou added the comment:

This new warning is introducing difficulties for some Cython-compiled modules: 
https://github.com/cython/cython/issues/1720

--
nosy: +pitrou

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Roundup Robot]

Roundup Robot added the comment:

New changeset 219c44fe8968 by Brett Cannon in branch 'default':
Issue #25791: Warn when __package__ != __spec__.parent.
https://hg.python.org/cpython/rev/219c44fe8968

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Brett Cannon]

Changes by Brett Cannon :


--
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Brett Cannon]

Brett Cannon added the comment:

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.

--
status: closed -> open

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Roundup Robot]

Roundup Robot added the comment:

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

--
nosy: +python-dev

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Brett Cannon]

Brett Cannon added the comment:

Thanks for the patch, Rose! I did notice your review comments about the missing 
goto and the stacklevel and I simply added them myself.

--
resolution:  -> fixed
stage: test needed -> resolved
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Rose Ames]

Rose Ames added the comment:

that's what I figured.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Rose Ames]

Rose Ames added the comment:

Patch with tests.  Not sure if importlib.h should be included?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Rose Ames]

Changes by Rose Ames :


--
keywords: +patch
Added file: http://bugs.python.org/file41609/issue25791.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Rose Ames]

Rose Ames added the comment:

Thanks for the quick review, new patch uploaded.

--
Added file: http://bugs.python.org/file41611/issue25791_2.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Brett Cannon]

Brett Cannon added the comment:

Including importlib.h doesn't hurt.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Brett Cannon]

Changes by Brett Cannon :


--
assignee:  -> brett.cannon

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue25791] Raise an ImportWarning when __spec__.parent/__package__ isn't defined for a relative import

[Rose Ames]

Changes by Rose Ames :


--
nosy: +superluser

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com