Opened 5 years ago
Closed 5 years ago
#23930 closed defect (fixed)
Doctest failures in Judson's abstract algebra book
Reported by:  mderickx  Owned by:  

Priority:  blocker  Milestone:  sage8.1 
Component:  doctest framework  Keywords:  
Cc:  rbeezer, vdelecroix  Merged in:  
Authors:  Maarten Derickx  Reviewers:  Vincent Delecroix 
Report Upstream:  N/A  Work issues:  
Branch:  e633e06 (Commits, GitHub, GitLab)  Commit:  e633e065cf33be46dd653b2ef9da04cfada03c23 
Dependencies:  Stopgaps: 
Description
See the logs of arando
sage t long src/sage/tests/books/judsonabstractalgebra/actionssage.py ********************************************************************** File "src/sage/tests/books/judsonabstractalgebra/actionssage.py", line 115, in sage.tests.books.judsonabstractalgebra.actionssage Failed example: A.orbits() Expected: [['000', '001', '010', '100', '011', '101', '110', '111']] Got: [['010', '011', '001', '100', '101', '000', '110', '111']] ********************************************************************** File "src/sage/tests/books/judsonabstractalgebra/actionssage.py", line 126, in sage.tests.books.judsonabstractalgebra.actionssage Failed example: S.list() Expected: [(), ('001','100','010')('011','101','110'), ('010','100')('011','101'), ('001','010','100')('011','110','101'), ('001','100')('011','110'), ('001','010')('101','110')] Got: [(), ('010','100')('011','101'), ('010','100','001')('011','110','101'), ('011','110')('001','100'), ('010','001','100')('011','101','110'), ('010','001')('110','101')] ********************************************************************** 1 item had failures: 2 of 35 in sage.tests.books.judsonabstractalgebra.actionssage [34 tests, 2 failures, 0.52 s] 
Since this is a machine with a lot of optional packages it is likely that it is caused by that. The results are mathematically correct, it is just that the order of the output changed.
Change History (8)
comment:1 Changed 5 years ago by
comment:2 Changed 5 years ago by
I'm reluctant to wrap output in sorted()
or similar in the textbook, it won't help with understanding. But I think I can rework the testing, perhaps including a hidden test of correctness that a reader won't see.
Maarten  might it be best to mark these random
for now, and I'll get proper replacements into the book on the next pass? The diffs in the Sage source will help me remember. This is the first test of how to handle when these fail, so let me know if you can think of a better approach.
I can review a change, but in less than a week I will soon be offline for an extended period.
comment:3 Changed 5 years ago by
Branch:  → public/23930 

Commit:  → e633e065cf33be46dd653b2ef9da04cfada03c23 
Marking as random till the next version of the book works for me. How are the files generated from the book?
New commits:
e633e06  Trac #23930: Fixed Doctest failures in Judson's abstract algebra book

comment:4 Changed 5 years ago by
Authors:  → Maarten Derickx 

Status:  new → needs_review 
comment:5 Changed 5 years ago by
Note that for the A.orbits()
test sorting will work. The S.list()
test is more complicated since there the same cycles are also notated in different ways.
comment:6 Changed 5 years ago by
Reviewers:  → Vincent Delecroix 

Status:  needs_review → positive_review 
The fix is good and non intrusive. It is now up to the book authors to find a more appropriate correction (if any is needed).
comment:7 Changed 5 years ago by
Thanks, Vincent. I ran tests overnight, so you just beat me to it on this one! I know it didn't really need a full run of the test suite, but I needed to practice my nascent git skills with the full Sage workflow. ;)
I'll likely clean up all the tests in the book again right around the end of the year, and get this cleaned up properly then.
Thanks, Maarten, also for shepherding this one through.
comment:8 Changed 5 years ago by
Branch:  public/23930 → e633e065cf33be46dd653b2ef9da04cfada03c23 

Resolution:  → fixed 
Status:  positive_review → closed 
This is now #23930