Closed 3f8450e1-87bf-41c6-ab53-29a0552debb3 closed 11 years ago
Done. (There is probably a minor conflict over one of the tableau patches I had above it in my queue.)
For patchbot:
Apply: trac_14266_ascii_art_13_05_15_EliX-jbp.patch, trac_14266-ascii_art-review-ts.patch
Changed dependencies from #8703 #14203 to #8703 #14203 #14574
I found that the fuzz was due to #14574, and is now based off of that patch.
For patchbot:
Apply: trac_14266_ascii_art_13_05_15_EliX-jbp.patch, trac_14266-ascii_art-review-ts.patch
The patchbot seems to have a bug that prevents it from seeing the first patch. I mentioned it to Robert.
Apply: trac_14266_ascii_art_13_05_15_EliX-jbp.patch trac_14266-ascii_art-review-ts.patch
Wait, now the patch applies without #14574 but when I add the new dependency then I get fuzz 2.
Maybe I uploaded the patch before I refreshed it...try this one. If not, then I guess we'll just remove #14574 as a dependency.
Apply: trac_14266_ascii_art_13_05_15_EliX-jbp.patch trac_14266-ascii_art-review-ts.patch
Now the review patch doesn't apply at all. Removing #14574 is also not an option since that patch has already been reviewed.
Sorry, accidentally applied against beta2 instead of beta3. The reviewer patch actually applies, but still only with fuzz 2.
Okay, this one should apply cleanly. If it doesn't, I'll give you my immortal soul. :P
Uh-oh, I still get fuzz 2 with sage-5.10.beta4. One immortal soul, please :-P
Now I'll have to go steal it back. It should me take precisely as long as I need to install 5.4.beta4
to get it to you.
Done. Sorry it took so long, I had to beat a guy at a fiddle contest for my immortal soul.
Looks good! You can have your soul back ;-)
I think most (all?) of the # not tested: works only in interactive shell
examples can actually be tested using a pexpect
interface, for example sage0
. I'm not going to set the ticket to needs_work for this, but it could be addressed in the future.
Hey Volker,
Thanks for doing the review and for returning my immortal soul. :D
Hey Jeroen,
I'm not quite sure what you mean, could you elaborate/give an example?
Thanks,
Travis
Needs to be rebased for sage-5.10.beta5
Doctest example:
sage: sage0.eval("%display ascii_art")
''
sage: sage0.eval("var('i')")
'i'
sage: print sage0.eval("sum(factorial(i)*x^i, i, 0, 10)")
10 9 8 7 6 5 4 3
3628800*x + 362880*x + 40320*x + 5040*x + 720*x + 120*x + 24*x + 6*x
2
+ 2*x + x + 1
Or, even better, use a test shell (does not require a separately spawned Sage session):
sage: from sage.misc.interpreter import get_test_shell
sage: shell = get_test_shell()
sage: shell.run_cell('%display ascii_art')
sage: shell.run_cell('var("i")')
sage: shell.run_cell('sum(factorial(i)*x^i, i, 0, 10)')
and so on.
Replying to @vbraun:
Needs to be rebased for sage-5.10.beta5
Are you sure? It applies perfectly fine on a first trial of sage-5.10.rc0
Attachment: trac_14266-ascii_art-review-ts.patch.gz
I've uploaded a new version of the review patch which uses the test shell. Since this changes doctests, and we (might) need to be rebased to 5.10.beta5
(which I'm building right now), I'm setting this back to needs review.
Ok, applies to plain sage-5.10.beta5. It does conflict with #14523, though.
Changes look good to me.
I just noticed that the sympy output is dependent on the terminal width:
sage -t sage/misc/ascii_art.py
**********************************************************************
File "sage/misc/ascii_art.py", line 45, in sage.misc.ascii_art
Failed example:
shell.run_cell('sum(factorial(i)*x^i, i, 0, 10)')
Expected:
10 9 8 7 6 5 4 3
3628800*x + 362880*x + 40320*x + 5040*x + 720*x + 120*x + 24*x + 6*x
<BLANKLINE>
2
+ 2*x + x + 1
Got:
10 9 8 7 6 5 4 3 2
3628800*x + 362880*x + 40320*x + 5040*x + 720*x + 120*x + 24*x + 6*x + 2*x + x + 1
**********************************************************************
File "sage/misc/ascii_art.py", line 67, in sage.misc.ascii_art
Description changed:
---
+++
@@ -22,3 +22,4 @@
- [attachment: trac_14266_ascii_art_13_05_15_EliX-jbp.patch](https://github.com/sagemath/sage-prod/files/10657365/trac_14266_ascii_art_13_05_15_EliX-jbp.patch.gz)
- [attachment: trac_14266-ascii_art-review-ts.patch](https://github.com/sagemath/sage-prod/files/10657366/trac_14266-ascii_art-review-ts.patch.gz)
+- [attachment: trac_14266_terminal_width.patch](https://github.com/sagemath/sage-prod/files/10657367/trac_14266_terminal_width.patch.gz)
Attachment: trac_14266_terminal_width.patch.gz
Initial patch
Last patch disables sympy terminal width detection during doctesting.
If you are happy with it, set ticket to positive review.
Patchbot:
apply trac_14266_ascii_art_13_05_15_EliX-jbp.patch, trac_14266-ascii_art-review-ts.patch, trac_14266_terminal_width.patch
Looks good to me. Thanks Volker.
Replying to @tscrim:
Looks good to me. Thanks Volker.
Beware though that trac_14266_terminal_width.patch introduces trailing whitespace.
If you don't like trailing whitespace then feel free to work on commit hooks that check for it.
Replying to @vbraun:
If you don't like trailing whitespace then feel free to work on commit hooks that check for it.
The sage dev scripts will strip them away for us. Which is how I actually detected this for I got a conflict when importing stuff into sage-git ... Do whatever you wish here; I just thought I might as well report the information.
In case anyone cares, here is the version without trailing whitespaces I put in the Sage-Combinat queue to ease my playing with importing into git.
http://combinat.sagemath.org/patches/file/tip/trac_14266_terminal_width.patch
Merged: sage-5.11.beta0
the ascii_art doesn't work with notebook. I purpose this patch but it is may be not a good solution.
Attachment: trac_14266_ascii_art_with_notebook.patch.gz
My patch creates a simple ascii-art module to manipule several structure: list,dict,tuple, linear expression:
Apply:
Depends on #8703 Depends on #14203 Depends on #14574
CC: @nthiery @VivianePons
Component: user interface
Keywords: ascii-art
Author: Jean-Baptiste Priez
Reviewer: Volker Braun, Travis Scrimshaw
Merged: sage-5.11.beta0
Issue created by migration from https://trac.sagemath.org/ticket/14266