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

from __future__ import <anything> results in a Syntax Error #7207

Closed
TimDumol mannequin opened this issue Oct 14, 2009 · 16 comments
Closed

from __future__ import <anything> results in a Syntax Error #7207

TimDumol mannequin opened this issue Oct 14, 2009 · 16 comments

Comments

@TimDumol
Copy link
Mannequin

TimDumol mannequin commented Oct 14, 2009

from future import * statements must be the first statements in a file. However, the old Sage Notebook inserts synchronization code before the file, and the new SageNB inserts prompt changing code first. Both of the aforementioned changes break the code.

CC: @qed777 @williamstein

Component: notebook

Author: Tim Dumol

Reviewer: Alex Leone

Merged: sagenb-0.7

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

@TimDumol TimDumol mannequin added this to the sage-4.3.2 milestone Oct 14, 2009
@TimDumol TimDumol mannequin assigned boothby Oct 14, 2009
@TimDumol
Copy link
Mannequin Author

TimDumol mannequin commented Oct 16, 2009

Fixed formatting to relocate future imports to the top of the file.

@TimDumol
Copy link
Mannequin Author

TimDumol mannequin commented Oct 16, 2009

Author: Tim Dumol

@TimDumol
Copy link
Mannequin Author

TimDumol mannequin commented Oct 16, 2009

comment:1

Attachment: trac_7207-sagenb-future-import.patch.gz

@TimDumol TimDumol mannequin added the s: needs review label Oct 16, 2009
@TimDumol
Copy link
Mannequin Author

TimDumol mannequin commented Dec 5, 2009

comment:2

Would anyone mind reviewing this? It just moves the displayhook_hack to another file, and adds some tests and a tad of refactoring.

@williamstein
Copy link
Contributor

comment:3

Needs work, since this has a subtle bug, which I bet you can easily fix. See below.

Using regular expressions is unfortunately not rock solid. For example, this input "mysteriously" gives a SyntaxError:

print """
from __future__ import division"""

outputs:

Syntax Error:
    from __future__ import division"""

whereas the similar

print """
from __xfuture__ import division"""

works fine.

I think the right fix is to require that the even in the notebook the from `future` import ... lines appear at the very top. This would make it possible to use the same method you already used.

@TimDumol
Copy link
Mannequin Author

TimDumol mannequin commented Dec 10, 2009

Uses stdlib's ast to parse the file instead of regexp's.

@TimDumol
Copy link
Mannequin Author

TimDumol mannequin commented Dec 10, 2009

comment:4

Attachment: trac_7207-sagenb-future-import.2.patch.gz

This version of the patch uses ast (from stdlib). It should prevent errors such as what you described.

@TimDumol TimDumol mannequin added s: needs review and removed s: needs work labels Dec 10, 2009
@TimDumol
Copy link
Mannequin Author

TimDumol mannequin commented Dec 16, 2009

Attachment: trac_7207-sagenb-future-import.2.2.patch.gz

Rebased on #7650 and its dependencies.

@TimDumol
Copy link
Mannequin Author

TimDumol mannequin commented Dec 16, 2009

Attachment: trac_7207-sagenb-future-import.2.3.patch.gz

Missed an import on the rebase.

@sagetrac-acleone
Copy link
Mannequin

sagetrac-acleone mannequin commented Jan 20, 2010

Attachment: trac_7207-sagenb-future-import.3.patch.gz

Rebased to ~0.60

@sagetrac-acleone
Copy link
Mannequin

sagetrac-acleone mannequin commented Jan 20, 2010

comment:5

Rebased to a bit after 0.60 (see trac_7207-sagenb-future-import.3.patch). Other than that LGTM.

@sagetrac-acleone
Copy link
Mannequin

sagetrac-acleone mannequin commented Jan 20, 2010

Reviewer: Alex Leone

@qed777
Copy link
Mannequin

qed777 mannequin commented Jan 25, 2010

comment:6

V4 is rebased for this queue:

sagenb-0.6
trac_7249-jinja2_v9.5.patch
trac_7962-link-worksheets-zip-file.patch
trac_7969-escaped-backslash.patch
trac_4217-html-system-formatting.3.patch
trac_3083-print-documentation.5.patch
trac_6182-double-quotes-ws.2.patch
trac_5263-publish-url.patch
trac_7631-republish-name.patch
trac_6353-cookies-diff-ports.patch
trac_7207-sagenb-future-import.3.patch

@qed777
Copy link
Mannequin

qed777 mannequin commented Jan 25, 2010

Rebased for SageNB 0.6 + queue in comment. Replaces previous.

@qed777
Copy link
Mannequin

qed777 mannequin commented Jan 25, 2010

Merged: sagenb-0.7

@qed777
Copy link
Mannequin

qed777 mannequin commented Jan 25, 2010

comment:7

Attachment: trac_7207-sagenb-future-import.4.patch.gz

@qed777 qed777 mannequin removed the s: positive review label Jan 25, 2010
@qed777 qed777 mannequin closed this as completed Jan 25, 2010
@qed777 qed777 mannequin mentioned this issue Feb 10, 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