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

When running .sage files, pass the preparsed file through a pipe #11821

Open
jhpalmieri opened this issue Sep 20, 2011 · 24 comments
Open

When running .sage files, pass the preparsed file through a pipe #11821

jhpalmieri opened this issue Sep 20, 2011 · 24 comments

Comments

@jhpalmieri
Copy link
Member

If you run "sage new.sage" on a script "new.sage", it creates a preparsed file "new.py". Then when a file in the twisted package tries to import the "new" Python module, it ends up importing this preparsed file instead, which leads to problems which can be hard to track down.

I can think of several solutions:

  1. hope that users will know not to use names like "new.sage". This is the current state of affairs, and it seems overly optimistic to me.
  2. give a warning, or fail outright with an error, if the name of the file is the same as that of a Python module.
  3. name the preparsed file something which is less likely to cause a clash, for example, turn "file.sage" into "file_preparsed.py" (see Give safer names to preparsed files #16955).
  4. don't save the preparsed file at all: write it to stdout, then run "sage-python -c " on it.

The attached patch implements number 4.


Apply

Depends on #71

Component: scripts

Author: John Palmieri

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

@jhpalmieri jhpalmieri added this to the sage-5.11 milestone Sep 20, 2011
@nexttime
Copy link
Mannequin

nexttime mannequin commented Sep 20, 2011

comment:2

I'd opt for either (2) or (3).

(3) is nicer from a user perspective, but may have other side effects.

(2) should be easy to implement (try: import ...), without potentially breaking other code because of assumptions on the filename, so I tend to prefer this variant.

P.S.: Looks like the patch is not yet based on #9739. (Haven't tried.)

If we change the way .sage files are preparsed when doctesting, we might run into errors due to left-over <file>.py files from attach() or load(). Just a thought.

@jhpalmieri
Copy link
Member Author

comment:3

Yet a fourth option, implemented in the "v2" patch: don't save the preparsed file at all. Here are the changes in this patch:

  • sage-preparse is split into two scripts: sage-preparse-one-file, which preparses a single file and writes the output to stdout. Then sage-preparse calls this, and saves the output to the appropriate place. Note that for sage-preparse, the help message in sage-sage says that it should "preparse file.sage and produce corresponding file_sage.py". This patch fixes both the behavior and the documentation: it saves to "file_sage.py", which should be a valid Python module name, instead of to "file.py" or "file.sage.py". This should help to prevent name clashes.

  • Then sage-run calls sage-preparse-one-file and feeds the output into "sage-python -c ", rather than feeding the preparsed py file as "sage-python ".

  • Independently of this patch, running "sage <file1.sage> <file2.sage> ..." acts just like running "sage <file1.sage>" -- it ignores the later arguments. So the documentation has been altered to match this.

  • On the other hand, sage-preparse can handle multiple files as inputs, so change its description (at the top of the file) to match.

To do: add some tests to sage/tests/cmdline.py.

@jhpalmieri
Copy link
Member Author

Dependencies: #9739, #10952, #8708

@jhpalmieri

This comment has been minimized.

@jhpalmieri

This comment has been minimized.

@jhpalmieri
Copy link
Member Author

comment:4

The new patch adds some doctests (not very sophisticated ones, but we can expand on them later).

@jhpalmieri
Copy link
Member Author

Changed dependencies from #9739, #10952, #8708 to #9739, #10952, #8708, #12069

@jhpalmieri
Copy link
Member Author

comment:5

This needs rebasing relative to #12069. I'll get to it eventually.

@jhpalmieri
Copy link
Member Author

Work Issues: rebase on #12069

@jhpalmieri
Copy link
Member Author

comment:6

Okay, now rebased.

@jhpalmieri
Copy link
Member Author

comment:7

Rebased to Sage 5.0.beta8 and #12069.

@jhpalmieri
Copy link
Member Author

Changed work issues from rebase on #12069 to none

@jhpalmieri

This comment has been minimized.

@roed314
Copy link
Contributor

roed314 commented Mar 14, 2013

comment:9

This needs to be rebased against #12415 (sage-doctest is no longer relevant).

@jhpalmieri
Copy link
Member Author

comment:10

Now rebased.

@jhpalmieri
Copy link
Member Author

root repo

@jhpalmieri
Copy link
Member Author

Attachment: trac_11821-root.patch.gz

Attachment: trac_11821-doctests.patch.gz

Sage library

@jhpalmieri
Copy link
Member Author

Attachment: trac_11821-preparse.v3.patch.gz

scripts repo

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@jdemeyer
Copy link

Changed dependencies from #9739, #10952, #8708, #12069 to #71

@jdemeyer
Copy link

comment:15

I don't like this patch because of #71. I regularly use the .py file when I get an exception to find out where the exception happened.

@jdemeyer jdemeyer changed the title sage-preparse: give 'safer' names to .py files When running .sage files, pass the preparsed file through a pipe Sep 10, 2014
@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

comment:18

See also #16955.

@jhpalmieri
Copy link
Member Author

comment:19

Should this be closed as "won't fix" because of #16955? Or is there a reason to preparse to stdout and pipe the results through sage?

@jdemeyer
Copy link

comment:20

Replying to @jhpalmieri:

Or is there a reason to preparse to stdout and pipe the results through sage?

Not having preparsed .py files cluttered around.

@mkoeppe mkoeppe removed this from the sage-6.4 milestone Dec 29, 2022
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

4 participants