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 set to public/23930
 Commit set to e633e065cf33be46dd653b2ef9da04cfada03c23
Hi Rob,
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
 Status changed from new to 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 set to Vincent Delecroix
 Status changed from needs_review to 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 changed from public/23930 to e633e065cf33be46dd653b2ef9da04cfada03c23
 Resolution set to fixed
 Status changed from positive_review to closed
This is now #23930