-
-
Notifications
You must be signed in to change notification settings - Fork 552
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use the Sage displayhook in doctests #16746
Comments
Branch: u/vbraun/sort_dicts_in_doctests |
New commits:
|
Commit: |
Author: Volker Braun |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
comment:5
The first commit changes the doctest displayhook to the one we want. This causes various doctest changes that need to be fixed. |
comment:8
I noticed that the fix for printing types "our way" (#14466) was incomplete, inside lists it still uses IPython style:
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
Dependencies: #16992 |
comment:11
I've refactored our displayhook machinery so we can pick & choose which parts we want to take from IPython. Now works ok, still have to fixup old doctests. |
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
|
This comment has been minimized.
This comment has been minimized.
comment:15
Wow, that is a massive set of changes... just out of curiosity, what does the documentation now look like for graphics? I don't know whether it is more obvious before or after this change that one needs to evaluate to get them. One reason for the spaces before (I mean the ones after plot doctests) was to "leave room" in the (live) doc for evaluating these. |
comment:16
I see why you needed #16992 now... |
comment:17
In
|
Branch pushed to git repo; I updated commit sha1. New commits:
|
comment:29
Now, all doctests pass on my linux mint 64 bit desktop. |
comment:30
All doctest pass on the buildbot... |
comment:31
Did you try this on ARM? |
comment:32
ARM is part of the buildbot now, so yes ;-) |
comment:33
Since it pass on the build bot and I don;t see any obvious problem with the new code itself I will give it a positive review. I expect ARM to be irrelevant in this context as we are only talking about re-formatting current doctests. |
Reviewer: François Bissey |
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
|
comment:36
surprisingly only one merge conflict ;-) |
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
|
comment:38
At this stage I think you have done all the work that should be done in one ticket. I suggest we have a follow up ticket for any new discoveries. |
Changed branch from u/vbraun/sort_dicts_in_doctests to |
The doctests output should look like the Sage commandline output, which it currently does not quite. Differences include:
{1,2}
instead ofset([1,2])
set()
instead ofset([])
<Foo at 0x...>
instead of<Foo object at 0x...>
Other changes:
__repr__
(IPython extension):Depends on #16992
CC: @dkrenn
Component: doctest framework
Author: Volker Braun
Branch/Commit:
3070715
Reviewer: François Bissey
Issue created by migration from https://trac.sagemath.org/ticket/16746
The text was updated successfully, but these errors were encountered: