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: | sage-8.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/judson-abstract-algebra/actions-sage.py ********************************************************************** File "src/sage/tests/books/judson-abstract-algebra/actions-sage.py", line 115, in sage.tests.books.judson-abstract-algebra.actions-sage 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/judson-abstract-algebra/actions-sage.py", line 126, in sage.tests.books.judson-abstract-algebra.actions-sage 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.judson-abstract-algebra.actions-sage [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