some absolute imports in the rings folder

[fchapoton]

convert some of the files in the rings folder to absolute import.

a small step towards py3

This involved untangling (partly) the import-hell in the rings folder.

CC: @jdemeyer @embray @tscrim

Component: python3

Keywords: days74

Author: Frédéric Chapoton

Branch: 5be6969

Reviewer: Jeroen Demeyer, Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/20629

[fchapoton]

Branch: public/20629

[fchapoton]

Commit: 0eb3d94

[fchapoton]

New commits:

0eb3d94

absolute import in the rings folder

[jdemeyer]

comment:2

Do we really need this in src/sage/rings/asymptotic/__init__.py and src/sage/rings/semirings/__init__.py, why aren't these files just empty?

from __future__ import absolute_import

from . import all

[sagetrac-git]

Changed commit from 0eb3d94 to 1b5b55f

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

1b5b55f

empty two `__init__` files

[jdemeyer]

comment:4

Style issue: better use

from .padic_base_leaves import (pAdicRingCappedRelative,
        pAdicRingCappedAbsolute, pAdicRingFixedMod,
        pAdicFieldCappedRelative)

instead of

from .padic_base_leaves import pAdicRingCappedRelative, \
                               pAdicRingCappedAbsolute, \
                               pAdicRingFixedMod, \
                               pAdicFieldCappedRelative

[jdemeyer]

comment:5

Question: why do you only consider Python (.py) files and not Cython (.pyx)? I'm NOT saying that you should do this in this ticket, just asking why...

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

c4a6297

trac 20629 style change

[sagetrac-git]

Changed commit from 1b5b55f to c4a6297

[fchapoton]

comment:7

Replying to @jdemeyer:

Question: why do you only consider Python (.py) files and not Cython (.pyx)? I'm NOT saying that you should do this in this ticket, just asking why...

oh, I just made a grep restricted to py files. Anyway, this is only a very partial change.

[jdemeyer]

comment:8

positive_review if this passes buildbot testing

[jdemeyer]

Reviewer: Jeroen Demeyer

[fchapoton]

comment:9

Hum, it seems that there is a problem with the correct python3 syntax

from . import rational

when the module rational is a cython module.

Is there some cython expert in the neighborhood ?

[jdemeyer]

comment:10

I don't know if it's really a Cython problem.

[fchapoton]

comment:11

maybe a circular dependency ?

[fchapoton]

comment:12

It seems that this involves some kind of circular import hell.

[jdemeyer]

comment:13

Probably, although it's not clear why something would fail as relative import but not as normal import...

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

b299e4e

Merge branch 'public/20629' into 7.3.b0

[sagetrac-git]

Changed commit from c4a6297 to b299e4e

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

daf689d

trac 20629 somewhat untangling the import hell in rings

[sagetrac-git]

Changed commit from b299e4e to daf689d

[fchapoton]

comment:16

After a long battling against the import hell in the rings folder, I think I have now a working branch.

Review, please !

[tscrim]

comment:23

One of the reasons why you might have started to see the import hell is that you imported a few things directly from the modules, rather than importing the entire module. In order to do the former, Python needs to evaluate the module at the moment you call the from foo import bar, whereas the latter is done when first calling foo.bar (not at import foo). This is why the "Pythonic" way is to import foo and then do foo.bar, but IMO, it just means we have to do more reading and/or work for maintenance.

[fchapoton]

comment:24

Hmm, not sure I understand your conclusion. Did I do something "wrong" ? Should we try to use import foo as much as possible, or the contrary ?

I just started wanting to convert to python3-compatible imports, and then had to resolve
a whole lot of circular imports. I did that by moving some imports inside functions, instead
of being on top of the files.

[tscrim]

comment:25

Replying to @fchapoton:

Hmm, not sure I understand your conclusion. Did I do something "wrong" ? Should we try to use import foo as much as possible, or the contrary ?

Well...there's my diagnosis of what might have caused the problem, and the "pythonic" treatment. However, I disagree with it being the best way to fix things.

I just started wanting to convert to python3-compatible imports, and then had to resolve
a whole lot of circular imports. I did that by moving some imports inside functions, instead
of being on top of the files.

I think you did the right thing. Moreover, this is in agreement with what we normally do in Sage. Does that clarify things?

[fchapoton]

comment:26

ping ?

[fchapoton]

comment:27

ping ?

[tscrim]

Changed reviewer from Jeroen Demeyer to Jeroen Demeyer, Travis Scrimshaw

[tscrim]

Changed keywords from none to days74

[vbraun]

comment:29

Sage doesn't start with patch

/home/release/Sage/local/lib/python2.7/site-packages/sage/rings/rational_field.py in <module>()
     41     True
     42 
     43 AUTHORS:
     44 
     45 - Niles Johnson (2010-08): :trac:`3893`: ``random_element()`` should pass on
     46   ``*args`` and ``**kwds``.
     47 
     48 - Travis Scrimshaw (2012-10-18): Added additional docstrings for full coverage.
     49   Removed duplicates of ``discriminant()`` and ``signature()``.
     50 
     51 """
     52 from __future__ import print_function, absolute_import
     53 
     54 from .rational import Rational
     55 from .integer import Integer
---> 56 from . import infinity
        global infinity = undefined
     57 ZZ = None
     58 
     59 from sage.structure.parent_gens import ParentWithGens
     60 import sage.rings.number_field.number_field_base as number_field_base
     61 from sage.misc.fast_methods import Singleton
     62 
     63 class RationalField(Singleton, number_field_base.NumberField):
     64     r"""
     65     The class ``RationalField`` represents the field `\QQ` of rational numbers.
     66 
     67     EXAMPLES::
     68 
     69         sage: a = long(901824309821093821093812093810928309183091832091)
     70         sage: b = QQ(a); b
     71         901824309821093821093812093810928309183091832091

ImportError: cannot import name infinity

[fchapoton]

comment:30

I do not see this. And two bots were green. Maybe a conflict with another ticket ?

[jdemeyer]

comment:31

I moved the working part of this branch to #20785. That reduces the problematic part, which can then be investigated more easily.

[jdemeyer]

comment:32

I suggest to wait until #20785 is merged and then look at this again.

[sagetrac-git]

Changed commit from b67f3d4 to 8297f23

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

2880ac8	Merge branch 'public/20629' into 7.3.b4
8297f23	remove double import

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

5be6969

trac 20629 fixing import of infinity in rational_field

[sagetrac-git]

Changed commit from 8297f23 to 5be6969

[fchapoton]

comment:36

bot is green ! ping ?

[vbraun]

Changed branch from public/20629 to 5be6969

[embray]

Changed commit from 5be6969 to none

[embray]

comment:39

Hah, wish I had seen this ticket earlier. I hit this the other day and thought it was just me. Thanks!