Skip to content
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

notebook -- spurious u0027's output #3154

Closed
williamstein opened this issue May 10, 2008 · 26 comments
Closed

notebook -- spurious u0027's output #3154

williamstein opened this issue May 10, 2008 · 26 comments

Comments

@williamstein
Copy link
Contributor

In the notebook we have this, caused by _eval_cmd in worksheet.py:

%python 
2+2
print "'hi\'"
///

'hi\u0027

Component: notebook

Author: Willem Jan Palenstijn, Tim Dumol

Reviewer: Mitesh Patel, Tim Dumol

Merged: sagenb-0.8

Issue created by migration from https://trac.sagemath.org/ticket/3154

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Jan 17, 2010

Attachment: trac_3154-spurious-u0027-output.patch.gz

Uses base64.b64en/decode instead.

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Jan 17, 2010

Author: Tim Dumol

@TimDumol TimDumol mannequin added the s: needs review label Jan 17, 2010
@wjp
Copy link
Mannequin

wjp mannequin commented Jan 17, 2010

comment:2

I think using %r to handle escaping quotes would be cleaner than using intermediate base64. I'm attaching a new patch that does this.

@wjp
Copy link
Mannequin

wjp mannequin commented Jan 17, 2010

Attachment: 3154_escaping_quotes.patch.gz

@qed777
Copy link
Mannequin

qed777 mannequin commented Jan 18, 2010

comment:3

Can we also use %r instead of base64-encoding in

  • sage.misc.preparser.load_wrap
  • sagenb.misc.support.automatic_name_filter
  • worksheet.Worksheet.preparse

?

@qed777
Copy link
Mannequin

qed777 mannequin commented Jan 18, 2010

comment:4

The second patch causes two SageNB doctest failures:

File "/home/tmp/sagenb-0.5/src/sagenb/sagenb/notebook/worksheet.py", line 3695:
    sage: W.check_for_system_switching(c1.cleaned_input_text(), c1)
Expected:
    (True, u"print _support_.syseval(gap, ur'''SymmetricGroup(5)''', '...')")
Got:
    (True, u"print _support_.syseval(gap, u'SymmetricGroup(5)', '/home/.sage/temp/chopin/5101/dir_2.sagenb/home/sage/0/cells/1')")
**********************************************************************
File "/home/tmp/sagenb-0.5/src/sagenb/sagenb/notebook/worksheet.py", line 3721:
    sage: W.check_for_system_switching(c1.cleaned_input_text(), c1)
Expected:
    (True, u"print _support_.syseval(gap, ur'''SymmetricGroup(5)''', '...')")
Got:
    (True, "print _support_.syseval(gap, u'SymmetricGroup(5)', '/home/.sage/temp/chopin/5101/dir_2.sagenb/home/sage/0/cells/1')")

Does the latter reveal a [minor] problem with #7249?

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Jan 18, 2010

comment:5

Replying to @qed777:

Can we also use %r instead of base64-encoding in

  • sage.misc.preparser.load_wrap
  • sagenb.misc.support.automatic_name_filter
  • worksheet.Worksheet.preparse

?

I believe so. I'd rather that be put in a new ticket though.

The attached patch should solve the mentioned doctest problems. Can't see how they're related to #7249 though.

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Jan 18, 2010

comment:6

Sorry, I forgot to attach the actual patch.

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Jan 18, 2010

Fixes a few doctests and a unicode encoding issue.

@qed777
Copy link
Mannequin

qed777 mannequin commented Jan 20, 2010

Attachment: 3154_escaping_quotes.2.patch.gz

Rebase for minor "hunk" failure. Replaces previous.

@qed777
Copy link
Mannequin

qed777 mannequin commented Jan 20, 2010

Reviewer: Mitesh Patel

@qed777
Copy link
Mannequin

qed777 mannequin commented Jan 20, 2010

Changed author from Tim Dumol to Willem Jan Palenstijn, Tim Dumol

@qed777
Copy link
Mannequin

qed777 mannequin commented Jan 20, 2010

comment:7

Attachment: 3154_escaping_quotes.3.patch.gz

Nice work! V3 is just a rebase of V2.

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Jan 20, 2010

Merged: sagenb-0.6

@TimDumol TimDumol mannequin removed the s: positive review label Jan 20, 2010
@TimDumol TimDumol mannequin closed this as completed Jan 20, 2010
@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 6, 2010

Rebased vs. SageNB 0.7.4. Replaces previous.

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 6, 2010

comment:9

Attachment: 3154_escaping_quotes.4.patch.gz

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 6, 2010

Work Issues: Rebase and fix tests

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 6, 2010

Changed merged from sagenb-0.6 to none

@qed777 qed777 mannequin added the s: needs work label Feb 6, 2010
@qed777 qed777 mannequin reopened this Feb 6, 2010
@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 6, 2010

comment:10

V4 is rebased against SageNB 0.7.4 (cf. #8051), but now several doctests fail, at least one of which I can't investigate lucidly right now. I'll return to this soon.

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 6, 2010

comment:11

I've reopened this ticket because its not in SageNB 0.7 (cf. #8051). I mistakenly thought, per hg log, that I had merged it, but I really merged #4217, whose commit string refers to #3154.

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 9, 2010

Doctest fixes. Replaces all previous.

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 9, 2010

comment:12

Attachment: 3154_escaping_quotes.5.patch.gz

V5 is rebased for SageNB 0.7.4 and it includes several new doctest fixes. Can someone review my changes?

@qed777
Copy link
Mannequin

qed777 mannequin commented Feb 9, 2010

Changed work issues from Rebase and fix tests to none

@qed777 qed777 mannequin added s: needs review and removed s: needs work labels Feb 9, 2010
@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Mar 19, 2010

comment:13

Doctests pass, no regressions noted.

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Mar 19, 2010

Changed reviewer from Mitesh Patel to Mitesh Patel, Tim Dumol

@loefflerd loefflerd mannequin modified the milestones: sage-4.3.1, sage-4.4 Apr 16, 2010
@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Apr 24, 2010

Merged: sagenb-0.8

@TimDumol TimDumol mannequin removed the s: positive review label May 4, 2010
@TimDumol TimDumol mannequin closed this as completed May 4, 2010
@TimDumol TimDumol mannequin mentioned this issue May 3, 2010
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants