Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#25371 closed enhancement (fixed)

py3: towards docbuild, more details

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.3
Component: python3 Keywords:
Cc: tscrim, jmantysalo Merged in:
Authors: Frédéric Chapoton Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: a9abcc8 (Commits) Commit:
Dependencies: Stopgaps:

Description


Change History (13)

comment:1 Changed 3 years ago by chapoton

  • Branch set to u/chapoton/25371
  • Cc tscrim jmantysalo added
  • Commit set to 6685b1737d91b1810e5f8e5e0189cf5575c96781
  • Status changed from new to needs_review

New commits:

6685b17py3: more details, towards docbuild

comment:2 Changed 3 years ago by jdemeyer

  • Status changed from needs_review to needs_info

What's wrong with

P = sum([arrow2d((0,0), (cos(x*0.1),sin(x*0.1)), hue=x/(20*pi)) for x in range(0,20*pi+1)])

comment:3 Changed 3 years ago by chapoton

[dochtml] [plotting ] During handling of the above exception, another exception occurred:
[dochtml] [plotting ] Traceback (most recent call last):
[dochtml] [plotting ] File "/home/chapoton/sage3/local/lib/python3.6/site-packages/matplotlib/sphinxext/plot_directive.py", line 525, in run_code
[dochtml] [plotting ] six.exec_(code, ns)
[dochtml] [plotting ] File "<string>", line 1, in <module>
[dochtml] [plotting ] File "sage/symbolic/expression.pyx", line 5721, in sage.symbolic.expression.Expression.__index__ (build/cythonized/sage/symbolic/expression.cpp:35714)
[dochtml] [plotting ] File "sage/symbolic/expression.pyx", line 1073, in sage.symbolic.expression.Expression._integer_ (build/cythonized/sage/symbolic/expression.cpp:9100)
[dochtml] [plotting ] TypeError: unable to convert 20*pi + 1 to an integer

comment:4 Changed 3 years ago by jdemeyer

Well, that's a genuine problem. Why shouldn't we no longer be allowed to use symbolic expressions in plots?

comment:5 Changed 3 years ago by jdemeyer

  • Branch changed from u/chapoton/25371 to u/jdemeyer/25371

comment:6 Changed 3 years ago by jdemeyer

  • Commit changed from 6685b1737d91b1810e5f8e5e0189cf5575c96781 to a9abcc892862794aaf7bd7268aafec729053e08f
  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_info to positive_review

I reverted that because it tries to fix the doctest instead of the bug.

The other change gets positive review.


New commits:

a9abcc8py3: more details, towards docbuild

comment:7 Changed 3 years ago by vbraun

  • Branch changed from u/jdemeyer/25371 to a9abcc892862794aaf7bd7268aafec729053e08f
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:8 Changed 3 years ago by chapoton

  • Commit a9abcc892862794aaf7bd7268aafec729053e08f deleted

Jeroen, see your own comment here: https://trac.sagemath.org/ticket/24758#comment:2

comment:9 Changed 3 years ago by chapoton

And the range behaviour itself is not working with python3. I do not think we should change that.

sage: list(range(20*pi+3))
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression._integer_ (build/cythonized/sage/symbolic/expression.cpp:9051)()

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression.pyobject (build/cythonized/sage/symbolic/expression.cpp:6090)()

TypeError: self must be a numeric expression

During handling of the above exception, another exception occurred:

TypeError                                 Traceback (most recent call last)
<ipython-input-1-3c6188e69b81> in <module>()
----> 1 list(range(Integer(20)*pi+Integer(3)))

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression.__index__ (build/cythonized/sage/symbolic/expression.cpp:35714)()

/home/chapoton/sage3/local/lib/python3.6/site-packages/sage/symbolic/expression.pyx in sage.symbolic.expression.Expression._integer_ (build/cythonized/sage/symbolic/expression.cpp:9100)()

TypeError: unable to convert 20*pi + 3 to an integer

comment:10 follow-ups: Changed 3 years ago by chapoton

Jeroen, would you accept this change that you refused here (in another ticket):

-        P = sum([arrow2d((0,0), (cos(x*0.1),sin(x*0.1)), hue=x/(20*pi)) for x in range(0,20*pi+1)])
+        P = sum([arrow2d((0,0), (cos(x*0.1),sin(x*0.1)), hue=x/(20*pi)) for x in range(floor(20*pi)+1)])

It seems to me that there is no issue in sage, only the fact that python3 is more strict on what it accepts inside range.

comment:11 in reply to: ↑ 10 Changed 3 years ago by jdemeyer

Replying to chapoton:

Jeroen, would you accept this change that you refused here (in another ticket):

-        P = sum([arrow2d((0,0), (cos(x*0.1),sin(x*0.1)), hue=x/(20*pi)) for x in range(0,20*pi+1)])
+        P = sum([arrow2d((0,0), (cos(x*0.1),sin(x*0.1)), hue=x/(20*pi)) for x in range(floor(20*pi)+1)])

That's not precisely the change that I rejected. You changed 20*pi by 63 originally in multiple places.

Anyway, let me figure out exactly what range() accepts on Python 2 and Python 3 and then I'll comment back.

comment:12 in reply to: ↑ 10 Changed 3 years ago by jdemeyer

Replying to chapoton:

It seems to me that there is no issue in sage, only the fact that python3 is more strict on what it accepts inside range.

I agree. Python 2 essentially replaces range(foo) by range(int(foo)). Python 3 requires an integer as argument to range().

comment:13 Changed 3 years ago by chapoton

Ok. I have made the ticket in #25578

Note: See TracTickets for help on using tickets.