Opened 9 years ago

Closed 8 years ago

Last modified 8 years ago

#14466 closed defect (fixed)

issue with type in doctests

Reported by: zimmerma Owned by: roed
Priority: major Milestone: sage-5.10
Component: doctest framework Keywords:
Cc: SimonKing 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:

Status badges

Description (last modified by jdemeyer)

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)

trac_14466_fix_type_repr.patch (714 bytes) - added by vbraun 9 years ago.
Initial patch
trac_14466-nb.patch (963 bytes) - added by nbruin 9 years ago.
include doctest

Download all attachments as: .zip

Change History (18)

comment:1 Changed 9 years ago by roed

There's a better workaround described in that thread: special case type in sage.misc.displayhook.format_obj.

comment:2 Changed 9 years ago by zimmerma

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 9 years ago by roed

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: Changed 9 years ago by 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).

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 9 years ago by roed

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 9 years ago by SimonKing

  • Cc SimonKing added

comment:7 in reply to: ↑ 4 Changed 9 years ago by nbruin

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?

Version 0, edited 9 years ago by nbruin (next)

Changed 9 years ago by vbraun

Initial patch

comment:8 Changed 9 years ago by vbraun

  • Authors set to Volker Braun
  • Status changed from new to needs_review

comment:9 Changed 9 years ago by vbraun

Anybody wants to review this trivial patch?

Changed 9 years ago by nbruin

include doctest

comment:10 Changed 9 years ago by nbruin

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 9 years ago by nbruin

patchbot apply trac_14466-nb.patch

so that we can see if this patch passes (Volker's does)

comment:12 Changed 9 years ago by vbraun

  • Authors changed from Volker Braun to Volker Braun, Nils Bruin
  • Description modified (diff)
  • Reviewers set to Nils Bruin, Volker Braun
  • Status changed from needs_review to positive_review

Looks good to me.

comment:13 Changed 8 years ago by jdemeyer

  • Merged in set to sage-5.10.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed

comment:14 Changed 8 years ago by vbraun

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 8 years ago by jdemeyer

  • Description modified (diff)

comment:16 Changed 8 years ago by jdemeyer

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.

Last edited 8 years ago by jdemeyer (previous) (diff)
Note: See TracTickets for help on using tickets.