Issue1648

classification
Title: Incomplete implementation of pep328 for relative imports
Type: behaviour Severity: normal
Components: Core Versions: 2.5.2b1
Milestone:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: zyasoft Nosy List: akruis, bpedman, zyasoft
Priority: Keywords: patch

Created on 2010-08-24.07:38:38 by akruis, last changed 2010-10-22.01:41:49 by zyasoft.

Files
File name Uploaded Description Edit Remove
add_regrtest_for_pep328.diff akruis, 2010-08-24.07:38:35 Adds new regrtest "test_import_pep328"
fixes_for_pep328.diff akruis, 2010-08-24.07:41:03 Fixes
Messages
msg6008 (view) Author: Anselm Kruis (akruis) Date: 2010-08-24.07:38:34
Hi,

Pep 328 introduces besides other features the notation of relative imports (from with dots) and "from __future__ import absolute_import". Unfortunately the jython implementation of both features is either incomplete (#1516) or broken (#1495).

Fortunately I need a fully functional pep328 implementation for my current project and could spend some time on this issue. 
The attached patch adds a new regression test "Lib/test/test_import_pep328.py". This test is pure python and also runs with c-python ("python25 test_import_pep328.py"). With c-python 2.5 the test runs OK. With the current (r7101) jython, several tests fail.

test_import_pep328.py contains two test classes: The first tests, how the parser and compiler convert import statements ("import" and "from ... import ...") into calls of the __import__ function. The second class tests, how the __import__ function handles various import situations utilizing the meta_path-hook to observe the inner workings of __import__.

I found the following defects.

- A coding bug in GrammarActions causing NPE

- Relative imports (from with at least one leading dot) call __import__ with a name containing the dots. That's probably wrong, because the information about the dots is in the level parameter of import and c-python strips the dots.

- CodeCompiler lacks support for absolute imports for "from A import *" and "import A".

- In case of an relative or absolute import (the current default), C-python calls __import__ with 4 arguments only to be compatible with previous implementations. Jython always uses the 5 argument form of __import__

- For relative imports, the computation of the full module name is broken.

Of course the test_import_pep328.py is intended to become a part of the jython test suite. I'm not sure, if I need to sign a PSF contributors agreement for it. If so, please let me know.
msg6009 (view) Author: Anselm Kruis (akruis) Date: 2010-08-24.07:41:03
And here are the fixes for the problems described above. The patch should apply clean against r7101.
msg6012 (view) Author: Jim Baker (zyasoft) Date: 2010-08-24.12:36:22
Very well written and comprehensive.
msg6013 (view) Author: Jim Baker (zyasoft) Date: 2010-08-24.13:14:36
Fixed in r7102
msg6188 (view) Author: Brandon (bpedman) Date: 2010-10-18.23:11:14
hmm, why is there no ability to re-open a bug? hopefully someone sees this comment...

I am getting an exception using 2.5.2b2 with this code. Specifically the cahnges to __builtin__.java where if the level is -1 (or < 0) it is not sending the full list of arguments to the import function. This may not cause problems with the built-in import but I am overriding the built-in __import__ function. 

When the cPickle module is loaded, it calls importModule("copy_reg") and  when this calls my import function I am missing the globals variable somehow because I get:

UnboundLocalError: local variable 'globals' referenced before assignment

Even though my import function is defined as:
def import_hook(name, globals=None, locals=None, fromlist=None, level= -1):

If I modify __builtins__.java to always pass the level argument I do not get this exception.
msg6195 (view) Author: Jim Baker (zyasoft) Date: 2010-10-22.01:41:49
See update in #1665 re Brandon's import problem, which is fixed in r7163
History
Date User Action Args
2010-10-22 01:41:49zyasoftsetmessages: + msg6195
2010-10-18 23:11:15bpedmansetnosy: + bpedman
messages: + msg6188
2010-08-24 13:14:51zyasoftsetstatus: open -> closed
resolution: accepted -> fixed
2010-08-24 13:14:36zyasoftsetmessages: + msg6013
title: pep328 related import problems + unittests + fix -> Incomplete implementation of pep328 for relative imports
2010-08-24 12:36:22zyasoftsetassignee: zyasoft
resolution: accepted
messages: + msg6012
nosy: + zyasoft
2010-08-24 07:41:05akruissetfiles: + fixes_for_pep328.diff
messages: + msg6009
2010-08-24 07:38:38akruiscreate