Opened 3 years ago
Closed 3 years 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, GitHub, GitLab) | Commit: | a4e1bed94546872411509738e272a690cd3b8627 |
Dependencies: | Stopgaps: |
Description
Change History (14)
comment:1 Changed 3 years ago by
- Branch set to u/chapoton/27668
- Cc dcoudert added
- Commit set to 2f24d0bc55ee4a53121b47ff373f1e0522f9c09e
- Status changed from new to needs_review
comment:2 Changed 3 years ago by
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 3 years ago by
postpone
comment:4 Changed 3 years ago by
- Reviewers set to David Coudert
- Status changed from needs_review to positive_review
So then, LGTM.
comment:5 Changed 3 years ago by
- 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 3 years ago by
this was comparing also the second argument "coloration" with itself, which is a dict..
comment:7 Changed 3 years ago by
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 3 years ago by
- Commit changed from 2f24d0bc55ee4a53121b47ff373f1e0522f9c09e to c8a1ab404602555ba05716bba376eb67812328aa
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c8a1ab4 | py3: fix some doctests in French book
|
comment:9 Changed 3 years ago by
- Status changed from needs_info to needs_review
done, thx for the suggestion
comment:10 Changed 3 years ago by
- Commit changed from c8a1ab404602555ba05716bba376eb67812328aa to a4e1bed94546872411509738e272a690cd3b8627
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
a4e1bed | py3: fix some doctests in French book
|
comment:11 Changed 3 years ago by
and the bot is now green
comment:12 Changed 3 years ago by
- Status changed from needs_review to positive_review
Right, it's much better like that.
comment:13 Changed 3 years ago by
+1 Thank you!
comment:14 Changed 3 years ago by
- Branch changed from u/chapoton/27668 to a4e1bed94546872411509738e272a690cd3b8627
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
py3: fix some doctests in French book