nondescript tracebacks with load and attach of .sage files

[Conrado PLG]

This is a followup to
http://groups.google.com/group/sage-devel/browse_thread/thread/c38b44514085fe4a

Sorry for ressurecting this, but I have investigated this issue a bit
and this is what I've found.

Somewhere between 4.3. and 4.3.5, the loading of .sage files have
changed (this only affects .sage files, not .py). Before, the file was
preparsed, written to a temp file and loaded with execfile. Now, the
file is preparsed to a string, and this string is loaded with exec.
Unfortunately, loading code with exec makes the traceback information
vanish for any code inside it.

This makes the debugging of .sage code a nightmare, since it is not
possible to know in which line an exception was thrown.

I am not sure how this could be fixed, maybe return to using temp
files?

I have changed preparser.py in order to do so, and it works. It is a
naive implementation but may be a quick fix for those who need it (it
preparses the .sage file and writes the result to a file with the same
name, appended with '.py', then execfile it). Search preparser.py for
this elif and change to the code below:

elif fpath.endswith('.sage'):
s = preparse_file(open(fpath).read())
f = open(fpath + '.py', 'w')
f.write(s)
f.close()
execfile(fpath + '.py', globals)


Conrado

[Maarten Derickx]

This change you describe sound like one that has been made for performance reasons. But since it obviously breaks tracebacks I think it should be either undone or made such that tracebacks work. There should at least be made a trac ticket for this issue.

[Dima Pasechnik]

one can have two modes, a debug one, with old the behaviour, and the performance one, with the new behaviour.

[leif]

On 17 Sep., 21:11, Dima Pasechnik <dimp...@gmail.com> wrote:
> one can have two modes, a debug one, with old the behaviour, and the
> performance one, with the new behaviour.

+1


-leif

[Marco Streng]

Thanks Conrado, that works perfectly. It is now ticket #11812.
http://trac.sagemath.org/sage_trac/ticket/11812

As for the efficiency: how big was the improvement here in efficiency?
Is this significant for load or for attach or both?

Can/should we make a distinction between load and attach?

If I do "attach", then that's because I am writing the code and trying
it out. That means I want to have good tracebacks always.

If I do "load", then code is loaded only once. That means that the
efficiency improvement becomes less important.

[Simon King]

Hi Marco,

On 18 Sep., 11:22, Marco Streng <marco.str...@gmail.com> wrote:
> If I do "attach", then that's because I am writing the code and trying
> it out. That means I want to have good tracebacks always.
>
> If I do "load", then code is loaded only once. That means that the
> efficiency improvement becomes less important.

Same here. So, I am +1 to your suggestion.

Cheers,
Simon

[Marco Streng]

On Sep 18, 12:18 pm, Simon King <simon.k...@uni-jena.de> wrote:
> Same here. So, I am +1 to your suggestion.

Thanks, but what was my suggestion?

I didn't write it very explicitly in that message, but I guess I
argued for going back to the old behaviour completely. If people
object to that, then an alternative suggestion is to go back to the
old behaviour for attach and to keep the high performance for load.

[William Stein]

There was some good reason for making the change (it fixed a bug?), so
somebody should look into that, right?

I'm pretty sure *I* made the change, but I can't remember why at this
moment.

-- william

>
> --
> To post to this group, send an email to sage-...@googlegroups.com
> To unsubscribe from this group, send an email to sage-devel+...@googlegroups.com
> For more options, visit this group at http://groups.google.com/group/sage-devel
> URL: http://www.sagemath.org
>

--
William Stein
Professor of Mathematics
University of Washington
http://wstein.org

[Marco Streng]

On Sep 18, 4:27 pm, William Stein <wst...@gmail.com> wrote:
> There was some good reason for making the change (it fixed a bug?), so
> somebody should look into that, right?
>
> I'm pretty sure *I* made the change, but I can't remember why at this
> moment.

Hi William,

You wrote the current version of that line here:
http://trac.sagemath.org/sage_trac/attachment/ticket/7514/sagelib-7514_combined.3.patch
(up to a very minor change made later by Willem Jan)

It is part of a major rewrite of load and attach and replaces an ugly
triplicate version. Could that have been the only reason?

Marco

ps 1 (offtopic). I found this out by trying a few cusom trac queries
and trac searches and reading quite a few of the search results. Is
there a more automated way of finding out in which ticket a piece of
code was changed?
ps 2. Conrado's fix does not break sage -t of sage/misc

[Conrado P. L. Gouvêa]

On Sun, Sep 18, 2011 at 12:29, Marco Streng <marco....@gmail.com> wrote:
> ps 2. Conrado's fix does not break sage -t of sage/misc

I should remark that the fix is very naive. It is probably not OK to
assume the directory containing the .sage file is writable. Sage 4.3
used to get the full path of the .sage file, replace '/' by '_' and
write it to a temp file. It should be easier just to port the older
code but I couldn't find where this is handled...

Conrado

[Marco Streng]

On Sep 18, 5:41 pm, Conrado P. L. Gouvêa <conrado...@gmail.com> wrote:
> Sage 4.3
> used to get the full path of the .sage file, replace '/' by '_' and
> write it to a temp file. It should be easier just to port the older
> code but I couldn't find where this is handled...

The function
sage.misc.interpreter.preparse_file_named
preparses a file to a termporary .py file and returns the name of the
temporary file

[Simon King]

Hi Marco,

On 18 Sep., 16:07, Marco Streng <marco.str...@gmail.com> wrote:
> On Sep 18, 12:18 pm, Simon King <simon.k...@uni-jena.de> wrote:
>
> > Same here. So, I am +1 to your suggestion.
>
> Thanks, but what was my suggestion?

Sorry, I thought your suggestion was that there should be a clear
traceback (hence, a temporary file) when you attach something, and
when you load something then it should be as efficient as possible,
hence, accepting less descriptive tracebacks.

Cheers,
Simon

[Nils Bruin]

On Sep 18, 8:29 am, Marco Streng <marco.str...@gmail.com> wrote:
> ps 1 (offtopic). I found this out by trying a few cusom trac queries
> and trac searches and reading quite a few of the search results. Is
> there a more automated way of finding out in which ticket a piece of
> code was changed?

Using "hg annotate" (or, more appropriately spelled, "hg blame") you
get a file listing with the last revision number that this line was
changed. In a good world, the commit message has the trac # in it.

For operations like this, the interactive "hg serve" (provides a local
webserver) is particularly convenient. You can step through different
revisions by just clicking.

[Marco Streng]

On 18 sep, 17:49, Simon King <simon.k...@uni-jena.de> wrote:
> Sorry, I thought your suggestion was that there should be a cleartraceback(hence, a temporary file) when youattachsomething, and

> when you load something then it should be as efficient as possible,
> hence, accepting less descriptive tracebacks.

Done. #11812 needs review.

[Marco Streng]

There is a problem with my patch (#11812). Can anyone help me?

I wanted to have a doctest in there that really tests whether the
traceback contains certain substrings. Python doctesting ignores the
content of a traceback. So to test the content of the traceback, I
tried starting a nested Sage session. "Sage" or "sage0" doesn't just
print the errors, but really throws them forward into the main Sage
session, so they don't help. With "import pexpect;
pexpect.spawn(full_path_to_sage_executable)", I get an error message.
The only thing that seemed to do exactly what I wanted was "import
pexpect; pexpect.spawn('sage')", but it doesn't, as the patchbot shows
at #11812.

For an example of what I was trying, see the patch at that ticket.

[Marco Streng]

New and easier patch is ready for review!

http://trac.sagemath.org/sage_trac/ticket/11812

The patch gives sage.misc.preparser.load an option preparse_to_file,
which defaults to True for attach and False for load. Preparsing to a
file gives good tracebacks, preparsing to memory gives keeps the speed
high.

The patch contains no complicated doctests, as the content of
tracebacks is too hard to test. So the patch is very short.

Marco