Opened 13 years ago

Closed 13 years ago

Last modified 13 years ago

#7514 closed enhancement (fixed)

rewrite load and attach

Reported by: William Stein Owned by: tbd
Priority: major Milestone: sage-4.3.1
Component: misc Keywords:
Cc: Sébastien Labbé, Robert Bradshaw, Dan Drake Merged in: sage-4.3.1.alpha0
Authors: William Stein Reviewers: Mitesh Patel
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by William Stein)

DEPENDS ON #7483

The goal of this ticket is to rewrite the load and attach commands to eliminate a lot of weirdness:

(1) they should both be usable as normal python functions, so people who find the load <...> form and attach <....> forms weird, can forget they exist. This also makes comprehensive doctesting way easier.

(2) it should be possible to do load DIR+'file.sage' say, and also constructions like

for i in range(3):
    load "setup%s.py"%i

The patches hopefully do the above and much more. They refactor all the relevant code, document it way, way better, clean it up, and unify it (there was tons of replication between the notebook and command line).

The notebook page (sagenb-7514.patch) is only needed if you use the ntoebook, by the way.

Attachments (11)

sagelib-7514.patch (31.8 KB) - added by William Stein 13 years ago.
apply to the core sage library
sagenb-7514.patch (1.8 KB) - added by William Stein 13 years ago.
sagelib_7514-part2.patch (738 bytes) - added by William Stein 13 years ago.
sagelib-7514_combined.patch (31.8 KB) - added by Mitesh Patel 13 years ago.
Combined sage repo patch. Depends on 4.3.alpha1, SageNB 0.4.6 (#7625), #7483, #7482.
sagenb-7514.2.patch (2.4 KB) - added by Mitesh Patel 13 years ago.
Fixed doctest failures. Replaces previous.
sagelib-7514_combined.2.patch (32.4 KB) - added by Mitesh Patel 13 years ago.
Fixed doctest failures. Depends on Sage 4.3, #7483, #7482. Replaces previous.
sagelib-7514_combined.3.patch (59.4 KB) - added by Mitesh Patel 13 years ago.
Fix(?) loading of .spyx files. Add preparser.py to reference manual. Replaces previous.
sagenb-7514-rebase.patch (2.4 KB) - added by William Stein 13 years ago.
rebased against sagenb-0.4.8 + trac #7483.
sagenb-7514-rebase-part2.patch (3.5 KB) - added by William Stein 13 years ago.
trac 7514 sagenb-part2: proper tracebacks; make source code introspection of functions defined in \ the notebook finally work in the notebook; properly cleanup worksheet files when notebook server is killed.
sagenb-7514-rebase-part3.patch (2.1 KB) - added by William Stein 13 years ago.
sagenb-7514-rebase-part3.2.patch (2.1 KB) - added by Mitesh Patel 13 years ago.
Trivial docstring tweaks. Replaces previous.

Download all attachments as: .zip

Change History (36)

Changed 13 years ago by William Stein

Attachment: sagelib-7514.patch added

apply to the core sage library

Changed 13 years ago by William Stein

Attachment: sagenb-7514.patch added

comment:1 Changed 13 years ago by William Stein

Description: modified (diff)
Status: newneeds_review

comment:2 Changed 13 years ago by Sébastien Labbé

Cc: Sébastien Labbé added
Report Upstream: N/A

Reading the summary of this ticket made me remember a problem I was having with the attach command : Can we attach a file already in the sage tree that we are editing? I already tried, but I stopped using it because sometimes the changes in the file were not considered and I have been stopping sage and running sage -br ever since then.

Maybe I was doing something bad or maybe this ticket solves the problem or maybe it is not possible at all to do this...??

Sébastien

comment:3 Changed 13 years ago by William Stein

Can we attach a file already in the sage tree that we are editing?

I stopped using it because sometimes the changes in the file were not

considered and I have been stopping sage and running sage -br ever since then.

You probably don't understand what attach does. All it does is execfile the file that you attached. There are situations where this happening might be perceived as "the file were not considered". E.g., if you create an install F of a class Foo defined in a file a.py, then a.py is reloaded, the object F is *not* somehow magically modified in memory to be an instance of a the new Foo that was defined in the file a.py. That's not how Python works, and it would be weird and confusing overall if things did work that way.

Changed 13 years ago by William Stein

Attachment: sagelib_7514-part2.patch added

comment:4 Changed 13 years ago by William Stein

Cc: Robert Bradshaw added

comment:5 in reply to:  2 Changed 13 years ago by William Stein

Status: needs_reviewneeds_work

Replying to slabbe:

Reading the summary of this ticket made me remember a problem I was having with the attach command : Can we attach a file already in the sage tree that we are editing? I already tried, but I stopped using it because sometimes the changes in the file were not considered and I have been stopping sage and running sage -br ever since then.

Maybe I was doing something bad or maybe this ticket solves the problem or maybe it is not possible at all to do this...??

You just have to *understand* what attach does. It reloads the file via execfile into the global namespace when the file changes. You can attach any file, in the tree or not. But imagine this: In the sage tree there is a file foo.py. There is another file bar.py that does "import foo" and uses some code in foo. If you attach foo.py (which results in exec'ing it in the global interpreter namespaces), then that will have no impact at all on bar.py.

Thus for some problems attach works very nicely for library code, and for others it doesn't. Which is which is clearer if you know what attach does.

comment:6 Changed 13 years ago by Dan Drake

Cc: Dan Drake added

Changed 13 years ago by Mitesh Patel

Attachment: sagelib-7514_combined.patch added

Combined sage repo patch. Depends on 4.3.alpha1, SageNB 0.4.6 (#7625), #7483, #7482.

comment:7 Changed 13 years ago by Mitesh Patel

I've attached a combined "sagelib" patch. It depends on 4.3.alpha1, SageNB 0.4.6 (#7625), #7483, and #7482.

Out of curiosity: What if we "overload" import by keeping a stack of all imported modules. When a watched file changes, we reload it and all modules loaded since the watched file was first loaded? Would this be useful in some situations?

comment:8 Changed 13 years ago by Mitesh Patel

With 4.3.alpha1 + #7625 + #7483 + #7482 + this ticket, several doctests failed:

        sage -t  devel/sage/sage/misc/sageinspect.py # 3 doctests failed
        sage -t  devel/sage/sage/misc/preparser.py # 10 doctests failed
        sage -t  devel/sage/sage/misc/reset.pyx # 2 doctests failed
        sage -t  devel/sage/sage/misc/session.pyx # 1 doctests failed

Should we stop doctesting most (all?) of sage/server?

comment:9 in reply to:  8 Changed 13 years ago by Mitesh Patel

Replying to mpatel:

With 4.3.alpha1 + #7625 + #7483 + #7482 + this ticket, several doctests failed:

Three tests fail in sagenb.misc.sageinpect. Please see #7650.

Should we stop doctesting most (all?) of sage/server?

Please see #7534.

comment:10 Changed 13 years ago by Mitesh Patel

I noticed

sage: f = 1
sage: save(f, 'f')
sage: attach('f.sobj')
Traceback (most recent call last)
...
ValueError: argument (='f.sobj') to load or attach must have extension py, sage, or pyx                             
sage: attached_files()
['f.sobj']

Questions about loading/attaching Cython files:

  • Can they only be loaded once, if they haven't changed? For example: If zzz.pyx contains print 'Zzz!', I see
    sage: load('zzz.pyx')
    Compiling zzz.pyx...
    Zzz!
    sage: load('zzz.pyx')
    sage: load('zzz.pyx')
    sage: # Now I edit zzz.pyx
    sage: load('zzz.pyx')
    Compiling zzz.pyx...
    Awake!
    
  • Can they access existing objects? For example: If I put
    try:
        b += 1
    except:
        b = 10
    print b
    

in a .pyx file, loading the file always sets b to 10, even if it's already defined.

Disclaimer: I'm still quite ignorant about Cython.

Changed 13 years ago by Mitesh Patel

Attachment: sagenb-7514.2.patch added

Fixed doctest failures. Replaces previous.

Changed 13 years ago by Mitesh Patel

Fixed doctest failures. Depends on Sage 4.3, #7483, #7482. Replaces previous.

comment:11 Changed 13 years ago by Mitesh Patel

Reviewers: Mitesh Patel
Status: needs_workneeds_review

The latest patches should fix the doctests and the attached_files problem mentioned above. I changed attach t to attach(t), since the doctesting framework appears to use a Python interpreter, i.e., not IPython, to run the examples. I also weakened [somewhat] the tests for preparser.modified_attached_files. Is there an IPython doctesting mode?

I'm not sure about the Cython file reload problem. Should we set use_cache=True when loading .pyx files explicitly?

On globals: I see that a Cython file foo.pyx is compiled to a module and loaded via from foo import *.

Can someone else please help to review this ticket?

Changed 13 years ago by Mitesh Patel

Fix(?) loading of .spyx files. Add preparser.py to reference manual. Replaces previous.

comment:12 Changed 13 years ago by William Stein

Description: modified (diff)

Changed 13 years ago by William Stein

Attachment: sagenb-7514-rebase.patch added

rebased against sagenb-0.4.8 + trac #7483.

Changed 13 years ago by William Stein

trac 7514 sagenb-part2: proper tracebacks; make source code introspection of functions defined in \ the notebook finally work in the notebook; properly cleanup worksheet files when notebook server is killed.

comment:13 Changed 13 years ago by William Stein

Right now the files that one must apply are:

To Sage Library:
 sagelib-7514_combined.3.patch 

To Notebook:
 sagenb-7514-rebase.patch  
 sagenb-7514-rebase-part2.patch

comment:14 Changed 13 years ago by William Stein

The file sagenb-7514-rebase-part2.patch fixes some serious issues. In particular, it makes it so we get proper tracebacks on errors, which was broken without this patch by the new architecture (of doing everything in the worksheet process, etc.). Also, source code introspection now works again for functions defined in the notebook, which many users wanted. Finally, there was a little bug where pressing control-c to stop the notebook server didn't result in quitting all worksheet processes, hence they left junk around.

comment:15 Changed 13 years ago by Mitesh Patel

Authors: William Stein
  • When I load/attach a .(s)pyx file in the notebook, ___code__.py appears in the cell's output HTML.
  • Minor: Should we make attached_files() in the notebook report, e.g.,
    [DATA+'foo.py']
    
    instead of
    ['/full/path/to/foo.py']
    
  • Typing f??<TAB> for a function f defined in the notebook shows its docstring but not its source code. Is this related to ?? not working for Cython functions defined in cells or attached DATA files?
  • Should we make it possible to edit attached .pyx files, just as we can edit .py, .sage, .spyx, and .txt files?
  • Minor: save_notebook appears to be called twice on shutdown. Is this a fail-safe behavior?

comment:16 Changed 13 years ago by William Stein

Status: needs_reviewneeds_work

When I load/attach a .(s)pyx file in the notebook, _code.py appears in the cell's output HTML.

That's a bug. I'll fix it soon.

Minor: Should we make attached_files() in the notebook report

I'm not sure. But I think this should be a separate ticket, since it would be a new feature.

  • Typing f??<TAB> for a function f defined in the notebook shows its docstring but > not its source code.

That's really weird, because it is one of the problems that this patch definitely fixes. Are you sure? Did you define a function f in one input cell, then do f?? in another, and it didn't show the source code? Were you using a clean sagenb-0.4.8 install for testing? Maybe I messed up posting the patch.

Minor: save_notebook appears to be called twice on shutdown. Is this a fail-safe behavior?

This has been the case forever. It is not caused by this patch. I don't think it is desirable. Please do open a ticket.

OK, so I'm going to fix the _code_.py issue you reported above, triple check that f?? works, and if not, figure out what went wrong, then mark this as "needs review" again.

comment:17 in reply to:  16 Changed 13 years ago by Mitesh Patel

Replying to was:

  • Typing f??<TAB> for a function f defined in the notebook shows its docstring but > not its source code.

That's really weird, because it is one of the problems that this patch definitely fixes. Are you sure? Did you define a function f in one input cell, then do f?? in another, and it didn't show the source code? Were you using a clean sagenb-0.4.8 install for testing? Maybe I messed up posting the patch.

It seems that just the last line of code is omitted. The source displayed is the preparsed source, but I don't know if this is a problem.

comment:18 Changed 13 years ago by William Stein

Status: needs_workneeds_review

The attached patch (sagenb-7514-rebase-part3.patch) should:

  1. Fixed this: "When I load/attach a .(s)pyx file in the notebook, _code.py appears in the cell's output HTML."
  1. Fixed this: "Typing f??<TAB> for a function f defined in the notebook shows its docstring but not its source code." Actually, this wasn't exactly broken... the last line of the docstring was missing. I've added a newline to the end of the input file, and now the problem is gone. This isn't a hack, since I think files are *supposed* to end with a newline, according to "IEEE Standard 1003.1 (aka POSIX)". Note that this patch *only* adds support for f?? for functions specifically entered in input normal cells (not cython, not load/attached). That's another issue.

Changed 13 years ago by William Stein

Changed 13 years ago by Mitesh Patel

Trivial docstring tweaks. Replaces previous.

comment:19 Changed 13 years ago by Mitesh Patel

Positive review.

If it's OK, I'll fix (e.g., INPUT/OUTPUT blocks) a few docstrings of functions affected by the sagelib patch.

comment:20 in reply to:  19 Changed 13 years ago by Mitesh Patel

Status: needs_reviewpositive_review

Replying to mpatel:

Positive review.

If it's OK, I'll fix (e.g., INPUT/OUTPUT blocks) a few docstrings of functions affected by the sagelib patch.

I'll make a separate ticket instead. We should include load and attach in the reference manual.

comment:21 Changed 13 years ago by Tim Dumol

The preparsing function introduced in #7483 lacks a "# -*- coding: utf-8 -*-" header that prevents the notebook from evaluating any string that contains unicode. This is addressed in #7835.

comment:22 Changed 13 years ago by Mike Hansen

I've merged sagelib-7514_combined.patch in sage-4.3.1.alpha0.

comment:23 Changed 13 years ago by William Stein

Resolution: fixed
Status: positive_reviewclosed

I merged the sagenb patches into sagenb-0.4.8.

comment:24 Changed 13 years ago by Karl-Dieter Crisman

Merged in: sage-4.3.1.alpha0

comment:25 Changed 13 years ago by Minh Van Nguyen

The attachment sagelib-7514_combined.3.patch in the Sage library.

Note: See TracTickets for help on using tickets.