#14466 closed defect (fixed)
issue with type in doctests
Reported by: | Paul Zimmermann | Owned by: | David Roe |
---|---|---|---|
Priority: | major | Milestone: | sage-5.10 |
Component: | doctest framework | Keywords: | |
Cc: | Simon King | Merged in: | sage-5.10.beta1 |
Authors: | Volker Braun, Nils Bruin | Reviewers: | Nils Bruin, Volker Braun |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
as mentioned on https://groups.google.com/forum/?hl=fr&fromgroups=#!topic/sage-devel/wuwWfhWnbCg, there is an issue with type
in doctests. When using type
in an interactive session we get for example:
---------------------------------------------------------------------- | Sage Version 5.8, Release Date: 2013-03-15 | | Type "notebook()" for the browser-based notebook interface. | | Type "help()" for help. | ---------------------------------------------------------------------- sage: type(ZZ) sage.rings.integer_ring.IntegerRing_class
However, when we put that in a doctest, we get an error:
tarte% cat /tmp/a.py def test(): """ EXAMPLE:: sage: type(ZZ) sage.rings.integer_ring.IntegerRing_class """ pass tarte% /localdisk/tmp/sage-5.8/sage -t /tmp/a.py sage -t "/tmp/a.py" ********************************************************************** File "/tmp/a.py", line 5: sage: type(ZZ) Expected: sage.rings.integer_ring.IntegerRing_class Got: <type 'sage.rings.integer_ring.IntegerRing_class'> ********************************************************************** 1 items had failures: 1 of 4 in __main__.example_0 ***Test Failed*** 1 failures. For whitespace errors, see the file /users/caramel/zimmerma/.sage//tmp/a_11238.py [2.1 s] ---------------------------------------------------------------------- The following tests failed: sage -t "/tmp/a.py" Total time for all tests: 2.2 seconds
A workaround is to put print type(...)
in the doctest, but this prints the form <type 'sage.rings.integer_ring.IntegerRing_class'>
which is not so nice...
apply trac_14466-nb.patch
Attachments (2)
Change History (18)
comment:1 Changed 10 years ago by
comment:2 Changed 10 years ago by
David, yes, however this workaround describes how to get <type ...>
in the interactive session.
We want the opposite: get sage.rings.integer_ring.IntegerRing_class
in the doctest, like the user gets in her/his interactive session.
Paul
comment:3 Changed 10 years ago by
I think that's a bad idea, since it means we have to change the result of a lot of doctests throughout Sage. Why not change both to be backward compatible?
comment:4 follow-up: 7 Changed 10 years ago by
David, maybe I should explain the context: in our book in french about Sage, we give examples of Sage code that are both printed in the book, and also used to generate doctests (to ensure those examples will still run with future versions of Sage).
Thus we want that the result "Got" in doctests matches what we get in an interactive session. If this means we should add a special instruction at the beginning of our doctests, it is fine for us. No need to change all Sage doctests.
Paul
comment:5 Changed 10 years ago by
Yeah, that sounds feasible. You could have a switch in the first few lines of the file that changes it to interactive mode? Another possibility would be to special case this comparison in the check_output()
function.
I'm going to sleep now and won't be online much in the next couple days.
comment:6 Changed 10 years ago by
Cc: | Simon King added |
---|
comment:7 Changed 10 years ago by
Replying to zimmerma:
David, maybe I should explain the context: in our book in french about Sage, we give examples of Sage code that are both printed in the book, and also used to generate doctests (to ensure those examples will still run with future versions of Sage).
Do you mean that book has already been printed? I'd say you can just update the book (immediately for the online version; in the next edition for a printed version, if that exists) to work correctly.
I think we've had a very small window where the new IPython has been printing different type
representations.
It's pretty clear to me that we want to ensure that our doctests test the same output as produced by an interactive session (they are documentation as well as tests!).
The string <type '...'>
is the straight str
of a type. Changing the doctest framework to test against anything else in a robust way means that we'd have to change the doctest framework so that any return value gets passed through the same print hooks that IPython offers.
The alternative seems much easier: Change IPython to (mostly) just produce the straight str
or repr
.
Thus we want that the result "Got" in doctests matches what we get in an interactive session.
Which we can accomplish by ensuring that the interactive session does give the same result as it did a while ago. Are you particularly married to this new, modified interactive result we're getting due to the IPython upgrade?
EDIT: In fact, in the latest version 1.0.9, your book on page 47 has
sage: o = 12/35 sage: type(o) <type 'sage.rings.rational.Rational'>
so I think you do want to go back to the old printing. We can easily do so by catching type
objects in sage.misc.displayhook.format_obj
. That unifies our printing again.
An additional reason to do so is that printing a type in the notebook still gives the normal <type '...'>
result as well. I think that really shows that we don't want the special IPython
behaviour in sage (doctest and notebook outvote IPython in my opinion).
comment:8 Changed 10 years ago by
Authors: | → Volker Braun |
---|---|
Status: | new → needs_review |
comment:10 Changed 10 years ago by
Turns out we can doctest this behaviour! See new patch. Also: if you compare print str('a')
and print repr('a')
you see that the latter is closer to what happens in the shell (not that it seems to matter for type objects)
Otherwise: of course a trivial patch (once you've dug out where to make the trivial change ...)
comment:11 Changed 10 years ago by
patchbot apply trac_14466-nb.patch
so that we can see if this patch passes (Volker's does)
comment:12 Changed 10 years ago by
Authors: | Volker Braun → Volker Braun, Nils Bruin |
---|---|
Description: | modified (diff) |
Reviewers: | → Nils Bruin, Volker Braun |
Status: | needs_review → positive_review |
Looks good to me.
comment:13 Changed 10 years ago by
Merged in: | → sage-5.10.beta1 |
---|---|
Resolution: | → fixed |
Status: | positive_review → closed |
comment:14 Changed 10 years ago by
Nils managed to generate this commit message in Sage's hg log:
changeset: 18302:4c8dabfafa75 user: Nils Bruin <nbruin@sfu.ca> date: Fri Apr 26 12:23:15 2013 -0700 summary: Trac #14466: diff --git a/sage/misc/displayhook.py b/sage/misc/displayhook.py
comment:15 Changed 10 years ago by
Description: | modified (diff) |
---|
comment:16 Changed 10 years ago by
There is also
changeset: 17832:42bc6caf1f2f user: Martin Raum <Martin.Raum@rwth-aachen.de> date: Tue Feb 19 10:32:37 2013 +0000 summary: Trac #13531: diff --git a/sage/quadratic_forms/quadratic_form__automorphisms.py b/sage/quadratic_forms/quadratic_form__automorphisms.py
Merger script fixed such that this won't happen again, but rewriting history to fix the bad messages is probably a bad idea.
There's a better workaround described in that thread: special case
type
insage.misc.displayhook.format_obj
.