py3: one fix in bdd_height.py

[chapoton]

plus some pep cleanup

[chapoton]

New commits:

​9eadc3d

py3: one fix in bdd_height

[chapoton]

failing doctest

[git]

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

​5c3ef67

trac 26619 using ceil instead of floor : better

[chapoton]

should be better now

[tscrim]

if RR(n*y).ceil() is n*y: # WHAT !?

+1

Although I am not convinced that

-        sage: enum_projective_number_field(X(K), bound=5^(1/3), prec=2^10)
+        sage: enum_projective_number_field(X(K), bound=RR(5^(1/3)), prec=2^10)

should be changed. It seems like you are loosing precision on bound by doing that (since 53 < 2¹⁰). You are doing that to avoid a doctest failure, right?

[chapoton]

I am doing that because of the "round" problem.. only hiding under the carpet the issue that "float" has no method "round"..

[git]

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

​5b1085b

trac 26619 pyflakes cleanup

[chapoton]

Bot is Green. So is this acceptable or not ?

[tscrim]

This is acceptable, but I want to see if we can quickly fix the underlying issue. Can you post the full traceback of the failure without your change? From looking at the code, I do not see where a round is called, but maybe I am missing something.

I am also seeing another possible py3 fix needed:

/home/travis/sage-build/local/lib/python2.7/site-packages/sage/rings/number_field/bdd_height.pyc in bdd_height(K, height_bound, tolerance, precision)
    535     possible_norm_set = set([])
    536     for n in range(class_number):
--> 537         for m in range(1, B+1):
    538             possible_norm_set.add(m*class_group_rep_norms[n])
    539     bdd_ideals = bdd_norm_pr_ideal_gens(K, possible_norm_set)

where B+1 needs to be cast to an int here.

[chapoton]

The best fix would be to add __round___ to our own real numbers.. See #25827 for the reasons why this seems to be difficult and nobody did it.

Here is a python3 traceback involving one of these range(1, B+1):

File "src/sage/schemes/projective/projective_space.py", line 1339, in sage.schemes.projective.projective_space.ProjectiveSpace_field.points_of_bounded_height
Failed example:
    len(list(P.points_of_bounded_height(bound=1.5, tolerance=0.1)))
Exception raised:
    Traceback (most recent call last):
      File "/home/u1/chapoton/sage3/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 671, in _run
        self.compile_and_execute(example, compiler, test.globs)
      File "/home/u1/chapoton/sage3/local/lib/python3.6/site-packages/sage/doctest/forker.py", line 1086, in compile_and_execute
        exec(compiled, globs)
      File "<doctest sage.schemes.projective.projective_space.ProjectiveSpace_field.points_of_bounded_height[4]>", line 1, in <module>
        len(list(P.points_of_bounded_height(bound=RealNumber('1.5'), tolerance=RealNumber('0.1'))))
      File "/home/u1/chapoton/sage3/local/lib/python3.6/site-packages/sage/schemes/projective/projective_space.py", line 1363, in points_of_bounded_height
        for x in iters: next(x) # put at zero
      File "/home/u1/chapoton/sage3/local/lib/python3.6/site-packages/sage/rings/number_field/bdd_height.py", line 537, in bdd_height
        for m in range(1, B+1):
    TypeError: 'sage.rings.real_mpfr.RealNumber' object cannot be interpreted as an integer

[tscrim]

Thank you.

Do we just want to cast int(B+1)? If you just want to keep things as they are, then you can set a positive review.

[chapoton]

sorry, but I do not understand your last remark. I would be happy enough with the current state.

[tscrim]

I meant changing

-for m in range(1, B+1):
+for m in range(1, int(B+1)):

[chapoton]

oh, well. We will see later. Let us move on here. I am setting to positive..