#15985 closed enhancement (fixed)
Python 3 preparation: Fix implicit relative imports (from sibling modules)
Reported by:  wluebbe  Owned by:  

Priority:  major  Milestone:  sage7.3 
Component:  python3  Keywords:  python3 
Cc:  tscrim, jmantysalo, jdemeyer  Merged in:  
Authors:  Wilfried Luebbe  Reviewers:  Jeroen Demeyer 
Report Upstream:  N/A  Work issues:  
Branch:  e2f59c2 (Commits)  Commit:  
Dependencies:  Stopgaps: 
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 metaticket ticket:15980.
Change History (21)
comment:1 Changed 7 years ago by
 Type changed from defect to enhancement
comment:2 Changed 6 years ago by
 Branch set to u/wluebbe/ticket/15985
 Commit set to 502ac5c98586116e74521bde30fb4c84edb507f5
 Status changed from new to needs_review
comment:3 Changed 6 years ago by
 Status changed from needs_review to needs_work
Ticket needs work :(
make failed while building htmldoc:
bytecompiling /home/wluebbe/sage6.2.beta4/local/lib/python2.7/sitepackages/sage/probability/random_variable.py to random_variable.pyc running install_egg_info Removing /home/wluebbe/sage6.2.beta4/local/lib/python2.7/sitepackages/sage6.2.beta5py2.7.egginfo Writing /home/wluebbe/sage6.2.beta4/local/lib/python2.7/sitepackages/sage6.2.beta5py2.7.egginfo real 10m2.009s user 35m33.530s sys 0m48.657s [ f local/etc/sagestarted.txt ]  local/bin/sagestarts build/pipestatus "./sage docbuild nopdflinks all html 2>&1" "tee a logs/dochtml.log" Traceback (most recent call last): File "/home/wluebbe/sage6.2.beta4/src/doc/common/builder.py", line 1465, in <module> import sage.all File "/home/wluebbe/sage6.2.beta4/local/lib/python2.7/sitepackages/sage/all.py", line 87, in <module> from sage.misc.all import * # takes a while File "/home/wluebbe/sage6.2.beta4/local/lib/python2.7/sitepackages/sage/misc/all.py", line 94, in <module> from .functional import (additive_order, File "/home/wluebbe/sage6.2.beta4/local/lib/python2.7/sitepackages/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/sage6.2.beta4/local/lib/python2.7/sitepackages/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/sage6.2.beta4/local/lib/python2.7/sitepackages/sage/rings/rational_field.py", line 54, in <module> from . import rational ImportError: cannot import name rational make: *** [dochtml] Error 1
The problem occurs while trying to import the cython module ration.pyx
. But rational.so
is in the same subdir. So the explicit relative import should succeed.
wluebbe@WillisUbuntu:~/sage6.2.beta4/local/lib/python2.7/sitepackages/sage/rings$ ls l ratio* rwrr 1 wluebbe wluebbe 28433 Apr 1 10:58 rational_field.py rwrr 1 wluebbe wluebbe 34240 Apr 1 11:08 rational_field.pyc rwxrxrx 1 wluebbe wluebbe 1666692 Apr 1 11:06 rational.so
comment:4 Changed 6 years ago by
 Commit changed from 502ac5c98586116e74521bde30fb4c84edb507f5 to cde9b0dc8d749e32ce47ce34f3cc1131e4d3ef6e
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
cde9b0d  local run of futurize w f absolute_import

comment:5 Changed 6 years ago by
Rebased on 6.2.beta6 and resolved merge conflicts.
comment:6 Changed 6 years ago by
 Branch changed from u/wluebbe/ticket/15985 to u/ohanar/relative_imports
 Commit changed from cde9b0dc8d749e32ce47ce34f3cc1131e4d3ef6e to 19692cba6ef085f5563f4302f34d6cb5c9a281e5
New commits:
19692cb  imports: fix a few automated mistakes

comment:7 Changed 6 years ago by
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 6 years ago by
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 6 years ago by
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 6 years ago by
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 6 years ago by
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 6 years ago by
 Milestone changed from sage6.2 to sage6.3
comment:13 Changed 6 years ago by
 Milestone changed from sage6.3 to sage6.4
comment:14 Changed 4 years ago by
 Milestone changed from sage6.4 to sage7.3
comment:15 Changed 4 years ago by
 Component changed from distribution to python3
comment:16 Changed 4 years ago by
Almost done, last step is #21036
there just remains a small problem in src/sage/misc/sagedoc.py
comment:17 followup: ↓ 21 Changed 4 years ago by
 Branch changed from u/ohanar/relative_imports to public/15985
 Cc tscrim jmantysalo jdemeyer added
 Commit changed from 19692cba6ef085f5563f4302f34d6cb5c9a281e5 to e2f59c294fe0dd027b981e1945f21ae97bad6dfb
 Status changed from needs_work to needs_review
comment:18 followup: ↓ 20 Changed 4 years ago by
 Reviewers set to Jeroen Demeyer
 Status changed from needs_review to positive_review
It's not really the last step, since there are still a lot of implicit relative imports in Cython files.
comment:19 Changed 4 years ago by
 Branch changed from public/15985 to e2f59c294fe0dd027b981e1945f21ae97bad6dfb
 Resolution set to fixed
 Status changed from positive_review to closed
comment:20 in reply to: ↑ 18 Changed 3 years ago by
 Commit e2f59c294fe0dd027b981e1945f21ae97bad6dfb deleted
comment:21 in reply to: ↑ 17 Changed 3 years ago by
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
.
New commits:
local run of futurize w f absolute_import