Opened 12 years ago

Closed 12 years ago

#11134 closed enhancement (fixed)

Add logging for Dokchitser scripts

Reported by: Jeroen Demeyer Owned by: William Stein
Priority: minor Milestone: sage-4.7.1
Component: interfaces Keywords: dokchitser gp
Cc: Merged in: sage-4.7.1.alpha0
Authors: Jeroen Demeyer Reviewers: John Cremona
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Jeroen Demeyer)

The attached patch adds a logfile for the Dokchitser gp scripts. This is something that I personally needed while debugging some issues with a new PARI package (#11130).

Attachments (1)

11134_dokchitser.patch (2.0 KB) - added by Jeroen Demeyer 12 years ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 12 years ago by Jeroen Demeyer

Status: newneeds_review

comment:2 Changed 12 years ago by John Cremona

Status: needs_reviewneeds_info

Applies fine t. 4.7.alpha4 and all tests pass. But I am a little concerned that with the patch, a log file is created whether or not the user wants one. This differs from the standard pexpect convention that a log file may be requested but is not created by default.

Would it work to move the logfile parameter to the init function, with default None, and have that function start up the interpreter?

John

comment:3 in reply to:  2 Changed 12 years ago by Jeroen Demeyer

Description: modified (diff)

Replying to cremona:

Applies fine t. 4.7.alpha4 and all tests pass. But I am a little concerned that with the patch, a log file is created whether or not the user wants one. This differs from the standard pexpect convention that a log file may be requested but is not created by default.

Let me just point out that gp (from PARI/GP) always logs by default (but this might be a bad thing too).

comment:4 in reply to:  2 Changed 12 years ago by Jeroen Demeyer

Replying to cremona:

Would it work to move the logfile parameter to the init function, with default None, and have that function start up the interpreter?

Not sure, because the Dokchitser stuff is often initialized indirectly.

I could live with the following:

logfile = None
# For debugging
#logfile = os.path.join(DOT_SAGE, 'dokchitser.log')
g = sage.interfaces.gp.Gp(script_subdirectory='dokchitser', logfile=logfile)

comment:5 Changed 12 years ago by John Cremona

I had not noticed that gp always produces a logfile! I would vote to remove that and make it optional.

I prefer your "debugging" option. This may well need revisiting after we have put in the new-style Dokchitser interface as worked on by Henri Cohen and me and Martin Raum recently -- I ost track of where that got to.

comment:6 Changed 12 years ago by Jeroen Demeyer

Status: needs_infoneeds_work

The best solution would be to have some kind of global "logging" option, similar to the "proof" option that we have currently in Sage. However, this is certainly a more complex solution and not high on my priority list.

Changed 12 years ago by Jeroen Demeyer

Attachment: 11134_dokchitser.patch added

comment:7 in reply to:  5 Changed 12 years ago by Jeroen Demeyer

Milestone: sage-4.7sage-4.7.1
Reviewers: John Cremona
Status: needs_workneeds_review

Replying to cremona:

I prefer your "debugging" option.

Done. I chose the simplest solution for this ticket, but you are right that there is work to be done concerning logging (but not on this ticket).

comment:8 Changed 12 years ago by John Cremona

Looks fine, testing now...

comment:9 in reply to:  8 Changed 12 years ago by John Cremona

Status: needs_reviewpositive_review

Replying to cremona:

Looks fine, testing now...

... and all passed.

comment:10 Changed 12 years ago by Jeroen Demeyer

Merged in: sage-4.7.1.alpha0
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.