Opened 11 years ago

Last modified 8 years ago

## #11821 needs_work defect

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

Reported by: | John Palmieri | Owned by: | |
---|---|---|---|

Priority: | major | Milestone: | sage-6.4 |

Component: | scripts | Keywords: | |

Cc: | Merged in: | ||

Authors: | John Palmieri | Reviewers: | |

Report Upstream: | N/A | Work issues: | |

Branch: | Commit: | ||

Dependencies: | #71 | Stopgaps: |

### Description (last modified by )

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:

- 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.
- give a warning, or fail outright with an error, if the name of the file is the same as that of a Python module.
- name the preparsed file something which is less likely to cause a clash, for example, turn "file.sage" into "file_preparsed.py" (see #16955).
- don't save the preparsed file at all: write it to stdout, then run "sage-python -c <PREPARSED>" on it.

The attached patch implements number 4.

Apply

- trac_11821-root.patch to the root repo
- trac_11821-preparse.v3.patch to the scripts repo
- trac_11821-doctests.patch to the main Sage repo

### Attachments (3)

### Change History (23)

### comment:1 Changed 11 years ago by

Status: | new → needs_review |
---|

### comment:2 Changed 11 years ago by

### comment:3 Changed 11 years ago by

Dependencies: | → #9739, #10952, #8708 |
---|---|

Description: | modified (diff) |

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 <OUTPUT>", rather than feeding the preparsed py file as "sage-python <FILE>".

- 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.

### comment:4 Changed 11 years ago by

Description: | modified (diff) |
---|

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

### comment:5 Changed 11 years ago by

Dependencies: | #9739, #10952, #8708 → #9739, #10952, #8708, #12069 |
---|---|

Status: | needs_review → needs_work |

Work issues: | → rebase on #12069 |

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

### comment:7 Changed 11 years ago by

Work issues: | rebase on #12069 |
---|

Rebased to Sage 5.0.beta8 and #12069.

### comment:8 Changed 11 years ago by

Description: | modified (diff) |
---|

### comment:9 Changed 10 years ago by

Status: | needs_review → needs_work |
---|

This needs to be rebased against #12415 (`sage-doctest`

is no longer relevant).

### comment:11 Changed 9 years ago by

Milestone: | sage-5.11 → sage-5.12 |
---|

### comment:12 Changed 9 years ago by

Milestone: | sage-6.1 → sage-6.2 |
---|

### comment:13 Changed 8 years ago by

Milestone: | sage-6.2 → sage-6.3 |
---|

### comment:14 Changed 8 years ago by

Milestone: | sage-6.3 → sage-6.4 |
---|

### comment:15 Changed 8 years ago by

Dependencies: | #9739, #10952, #8708, #12069 → #71 |
---|

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.

### comment:16 Changed 8 years ago by

Status: | needs_review → needs_work |
---|---|

Summary: | sage-preparse: give 'safer' names to .py files → When running .sage files, pass the preparsed file through a pipe |

### comment:17 Changed 8 years ago by

Description: | modified (diff) |
---|

### comment:19 follow-up: 20 Changed 8 years ago by

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?

### comment:20 Changed 8 years ago by

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.

**Note:**See TracTickets for help on using tickets.

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

(3) is nicer from a user perspective, but

mayhave 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.