Opened 8 months ago

Closed 8 months ago

#27668 closed enhancement (fixed)

py3: fix some doctests in French book (graph and programming)

Reported by: chapoton Owned by:
Priority: major Milestone: sage-8.8
Component: python3 Keywords:
Cc: dcoudert Merged in:
Authors: Frédéric Chapoton Reviewers: David Coudert
Report Upstream: N/A Work issues:
Branch: a4e1bed (Commits) Commit: a4e1bed94546872411509738e272a690cd3b8627
Dependencies: Stopgaps:

Description


Change History (14)

comment:1 Changed 8 months ago by chapoton

  • Branch set to u/chapoton/27668
  • Cc dcoudert added
  • Commit set to 2f24d0bc55ee4a53121b47ff373f1e0522f9c09e
  • Status changed from new to needs_review

New commits:

2f24d0bpy3: fix some doctests in French book

comment:2 Changed 8 months ago by dcoudert

The proposed patch fixes some doctests, but I also see this error.

File "src/sage/tests/books/computational-mathematics-with-sagemath/graphtheory_doctest.py", line 386, in sage.tests.books.computational-mathematics-with-sagemath.graphtheory_doctest
Failed example:
    for i in tasks:
      print("t{} assigned to {}".format(i,M.neighbors('t'+str(i))[0]))
Expected:
    t0 assigned to w2
    t1 assigned to w3
    t2 assigned to w5
    t3 assigned to w8
    t4 assigned to w1
    t5 assigned to w7
    t6 assigned to w9
    t7 assigned to w0
    t8 assigned to w4
    t9 assigned to w6
Got:
    t0 assigned to w3
    t1 assigned to w9
    t2 assigned to w5
    t3 assigned to w0
    t4 assigned to w1
    t5 assigned to w2
    t6 assigned to w8
    t7 assigned to w7
    t8 assigned to w4
    t9 assigned to w6

Do you want to solve it it or to postpone it for another ticket ?

comment:3 Changed 8 months ago by chapoton

postpone

comment:4 Changed 8 months ago by dcoudert

  • Reviewers set to David Coudert
  • Status changed from needs_review to positive_review

So then, LGTM.

comment:5 Changed 8 months ago by embray

  • Status changed from positive_review to needs_info

What's up with this?

@@ -280,8 +280,11 @@ Sage example in ./graphtheory.tex, line 1736::
 Sage example in ./graphtheory.tex, line 1746::
 
   sage: P = Permutations(range(g.order()))
-  sage: n_colors, coloration = min(
-  ....:    greedy_coloring(g, P.random_element()) for i in range(50))
+  sage: n_colors = g.order()
+  sage: for i in range(50):
+  ....:     n_col, coloration = greedy_coloring(g, P.random_element())
+  ....:     if n_col < n_colors:
+  ....:         n_colors = n_col
   sage: n_colors
   4

It's not clear to me why the old code wouldn't work on Python 3. In any case this is an ugly way to write min(...) so I would rather just skip this test if for some reason it doesn't work as a 1-liner.

Please in the future CC me on changes to these tests.

comment:6 Changed 8 months ago by chapoton

this was comparing also the second argument "coloration" with itself, which is a dict..

comment:7 Changed 8 months ago by embray

min() takes a key argument much like sorted so you can take the min while comparing just n_colors like min(..., key=lambda c: c[0]), for example.

comment:8 Changed 8 months ago by git

  • Commit changed from 2f24d0bc55ee4a53121b47ff373f1e0522f9c09e to c8a1ab404602555ba05716bba376eb67812328aa

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

c8a1ab4py3: fix some doctests in French book

comment:9 Changed 8 months ago by chapoton

  • Status changed from needs_info to needs_review

done, thx for the suggestion

comment:10 Changed 8 months ago by git

  • Commit changed from c8a1ab404602555ba05716bba376eb67812328aa to a4e1bed94546872411509738e272a690cd3b8627

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

a4e1bedpy3: fix some doctests in French book

comment:11 Changed 8 months ago by chapoton

and the bot is now green

comment:12 Changed 8 months ago by dcoudert

  • Status changed from needs_review to positive_review

Right, it's much better like that.

comment:13 Changed 8 months ago by embray

+1 Thank you!

comment:14 Changed 8 months ago by vbraun

  • Branch changed from u/chapoton/27668 to a4e1bed94546872411509738e272a690cd3b8627
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.