Opened 9 years ago

Closed 6 years ago

Last modified 5 years ago

#15985 closed enhancement (fixed)

Python 3 preparation: Fix implicit relative imports (from sibling modules)

Reported by: Wilfried Luebbe Owned by:
Priority: major Milestone: sage-7.3
Component: python3 Keywords: python3
Cc: Travis Scrimshaw, Jori Mäntysalo, Jeroen Demeyer Merged in:
Authors: Wilfried Luebbe Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: e2f59c2 (Commits, GitHub, GitLab) Commit:
Dependencies: Stopgaps:

Status badges

Description

Python 3 accepts only absolute imports and explicit relative imports. To be Python 2 compatible all affected modules need from __future__ import absolute_import.

lib2to3/fixes/fix_import.py does not add this future import.
Therefore we intend to use an enhanced fixer from https://pypi.python.org/pypi/future/0.11.3.
The latest development version also handles imports of pyx modules.

Changes according to libfuturize/fixes/fix_absolute_imports.py:

If spam is being imported from the local directory, this import:
    from spam import eggs
becomes:
    from .spam import eggs

and this import:
    import spam
becomes:
    from . import spam

This ticket is tracked as a dependency of meta-ticket ticket:15980.

Change History (21)

comment:1 Changed 9 years ago by Wilfried Luebbe

Type: defectenhancement

comment:2 Changed 8 years ago by Wilfried Luebbe

Branch: u/wluebbe/ticket/15985
Commit: 502ac5c98586116e74521bde30fb4c84edb507f5
Status: newneeds_review

New commits:

502ac5clocal run of futurize -w -f absolute_import

comment:3 Changed 8 years ago by Wilfried Luebbe

Status: needs_reviewneeds_work

Ticket needs work :-(

make failed while building html-doc:

byte-compiling /home/wluebbe/sage-6.2.beta4/local/lib/python2.7/site-packages/sage/probability/random_variable.py to random_variable.pyc
running install_egg_info
Removing /home/wluebbe/sage-6.2.beta4/local/lib/python2.7/site-packages/sage-6.2.beta5-py2.7.egg-info
Writing /home/wluebbe/sage-6.2.beta4/local/lib/python2.7/site-packages/sage-6.2.beta5-py2.7.egg-info

real	10m2.009s
user	35m33.530s
sys	0m48.657s
[ -f local/etc/sage-started.txt ] || local/bin/sage-starts
build/pipestatus "./sage --docbuild --no-pdf-links all html  2>&1" "tee -a logs/dochtml.log"
Traceback (most recent call last):
  File "/home/wluebbe/sage-6.2.beta4/src/doc/common/builder.py", line 1465, in <module>
    import sage.all
  File "/home/wluebbe/sage-6.2.beta4/local/lib/python2.7/site-packages/sage/all.py", line 87, in <module>
    from sage.misc.all       import *         # takes a while
  File "/home/wluebbe/sage-6.2.beta4/local/lib/python2.7/site-packages/sage/misc/all.py", line 94, in <module>
    from .functional import (additive_order,
  File "/home/wluebbe/sage-6.2.beta4/local/lib/python2.7/site-packages/sage/misc/functional.py", line 37, in <module>
    from sage.rings.complex_double import CDF
  File "integer.pxd", line 9, in init sage.rings.complex_double (sage/rings/complex_double.c:17880)
  File "integer.pyx", line 179, in init sage.rings.integer (sage/rings/integer.c:40148)
  File "/home/wluebbe/sage-6.2.beta4/local/lib/python2.7/site-packages/sage/rings/infinity.py", line 203, in <module>
    import sage.rings.rational
  File "fast_arith.pxd", line 5, in init sage.rings.rational (sage/rings/rational.c:29096)
  File "fast_arith.pyx", line 51, in init sage.rings.fast_arith (sage/rings/fast_arith.c:8036)
  File "integer_ring.pyx", line 65, in init sage.rings.integer_ring (sage/rings/integer_ring.c:12457)
  File "/home/wluebbe/sage-6.2.beta4/local/lib/python2.7/site-packages/sage/rings/rational_field.py", line 54, in <module>
    from . import rational
ImportError: cannot import name rational
make: *** [doc-html] Error 1

The problem occurs while trying to import the cython module ration.pyx. But rational.sois in the same subdir. So the explicit relative import should succeed.

wluebbe@Willis-Ubuntu:~/sage-6.2.beta4/local/lib/python2.7/site-packages/sage/rings$ ls -l ratio*
-rw-r--r-- 1 wluebbe wluebbe   28433 Apr  1 10:58 rational_field.py
-rw-r--r-- 1 wluebbe wluebbe   34240 Apr  1 11:08 rational_field.pyc
-rwxr-xr-x 1 wluebbe wluebbe 1666692 Apr  1 11:06 rational.so

comment:4 Changed 8 years ago by git

Commit: 502ac5c98586116e74521bde30fb4c84edb507f5cde9b0dc8d749e32ce47ce34f3cc1131e4d3ef6e

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

cde9b0dlocal run of futurize -w -f absolute_import

comment:5 Changed 8 years ago by Wilfried Luebbe

Rebased on 6.2.beta6 and resolved merge conflicts.

comment:6 Changed 8 years ago by R. Andrew Ohana

Authors: Wilfried Luebbe
Branch: u/wluebbe/ticket/15985u/ohanar/relative_imports
Commit: cde9b0dc8d749e32ce47ce34f3cc1131e4d3ef6e19692cba6ef085f5563f4302f34d6cb5c9a281e5

New commits:

19692cbimports: fix a few automated mistakes

comment:7 Changed 8 years ago by R. Andrew Ohana

So it looks like the issue that you are getting into is the wonder that is affectionately referred to as Sage's import hell :).

Looking at the traceback, it looks like the issue is a circular import that mysteriously works right now: sage.rings.rational imports something from sage.rings.fast_arith which imports something from sage.rings.integer_ring which imports something from sage.rings.rational_field which imports sage.rings.rational.

This is one of the big reasons why Sage is not truly usable as a python library, because the order in which you import Sage is incredibly delicate, and seems to magically work (and as you can see from this patch which shouldn't change anything, it truly is magical). There hasn't been much effort to fix this, but I would say that getting Python 3 working is a pretty big motivator.

comment:8 Changed 8 years ago by R. Andrew Ohana

One thing to try (which still might break) is removing all relative imports in favor of absolute imports, rather than just converting them to explicit relative imports.

comment:9 Changed 8 years ago by Wilfried Luebbe

Some numbers:

lines with "import" 26,3231
- thereof lines with "from sage" 19,581
- thereof lines with "import sage" 1,650
leaving lines with imports from Python
standard library and implicit relative import
(and 1 explicit relative import :-)
about 5,000

Questions:

  • Is there agreement in the Sage community that absolute import is the way to go?
  • Can it help with the above ImportError? And similar ones?
  • Other benefits or drawbacks?

comment:10 Changed 8 years ago by R. Andrew Ohana

As your numbers indicate (and from my personal experience), there is definitely an unofficial convention to use absolute from..import statements in the sage library, i.e. from sage.foo import bar; however this is one of (if not the main) culprit behind the circular import issues of the Sage library. A lot of the circular import issues could be resolved by just importing a module, and not particular attributes of the module (i.e. using import sage.foo or import sage.foo as bar and using sage.foo.baz/bar.baz instead of from sage.foo import baz).

comment:11 Changed 8 years ago by Wilfried Luebbe

Can I read this proposal as:
Do the (absolute) imports as import sage.somewhere.amodule as am and then use it as am.a_function(a_parm).

Perhaps the module could recommend its "standard" abbreviation (here am). Such a convention might help while reading code that uses the module.

comment:12 Changed 8 years ago by For batch modifications

Milestone: sage-6.2sage-6.3

comment:13 Changed 8 years ago by For batch modifications

Milestone: sage-6.3sage-6.4

comment:14 Changed 6 years ago by Frédéric Chapoton

Milestone: sage-6.4sage-7.3

Let us do that chunk by chunk.

See #20958, #20960, #20955, #20945, #20947 for the first steps

Last edited 6 years ago by Frédéric Chapoton (previous) (diff)

comment:15 Changed 6 years ago by Frédéric Chapoton

Component: distributionpython3

comment:16 Changed 6 years ago by Frédéric Chapoton

Almost done, last step is #21036

there just remains a small problem in src/sage/misc/sagedoc.py

comment:17 Changed 6 years ago by Frédéric Chapoton

Branch: u/ohanar/relative_importspublic/15985
Cc: Travis Scrimshaw Jori Mäntysalo Jeroen Demeyer added
Commit: 19692cba6ef085f5563f4302f34d6cb5c9a281e5e2f59c294fe0dd027b981e1945f21ae97bad6dfb
Status: needs_workneeds_review

Here is the very small last step. Hopefully the doctests will pass.

For the reviewer: to check, see that (after #21036 is closed)

futurize -w -f absolute_import src/sage

does nothing.


New commits:

e2f59c2trac 15985 last old-style import in misc

comment:18 Changed 6 years ago by Jeroen Demeyer

Reviewers: Jeroen Demeyer
Status: needs_reviewpositive_review

It's not really the last step, since there are still a lot of implicit relative imports in Cython files.

comment:19 Changed 6 years ago by Volker Braun

Branch: public/15985e2f59c294fe0dd027b981e1945f21ae97bad6dfb
Resolution: fixed
Status: positive_reviewclosed

comment:20 in reply to:  18 Changed 5 years ago by Jeroen Demeyer

Commit: e2f59c294fe0dd027b981e1945f21ae97bad6dfb

Replying to jdemeyer:

It's not really the last step, since there are still a lot of implicit relative imports in Cython files.

See #22806 and #22808.

comment:21 in reply to:  17 Changed 5 years ago by Jeroen Demeyer

Replying to chapoton:

Here is the very small last step.

How did you determine that this is the "last step"? In other words, how did you determine which modules need to be fixed?

There are still plenty of .py and .pyx files which do not have from __future__ import absolute_import.

Note: See TracTickets for help on using tickets.