Opened 13 years ago

Closed 13 years ago

#2901 closed defect (fixed)

[with patch; with positive review] rewrite load_session and save_session to use much better modern techniques (in particular merge the notebook and non-notebook implementations)

Reported by: was Owned by: cwitty
Priority: major Milestone: sage-3.0
Component: misc Keywords:
Cc: Merged in:
Authors: Reviewers:
Report Upstream: Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

Basically, I know a lot more about how to write Python/Cython? code than I used to, so i can write these functions in a way that is (1) vastly better, and (2) will be easily doctested, and (3) work in (almost) the same way in the notebook or command line.

This depends on the patch up at #2883. #2883 should be applied then the code attached to this patch, once this patch is accepted.

I've separated this out from #2883, since #2883 really *must* get applied, and the code here should be, but it's really a separate issue, and more nontrivial.

Attachments (1)

sage-2901.patch (23.2 KB) - added by was 13 years ago.

Download all attachments as: .zip

Change History (5)

Changed 13 years ago by was

comment:1 Changed 13 years ago by was

  • Summary changed from rewrite load_session and save_session to use much better modern techniques (in particular merge the notebook and non-notebook implementations) to [with patch; needs review] rewrite load_session and save_session to use much better modern techniques (in particular merge the notebook and non-notebook implementations)

Patch is up. This basically replaces a bunch of terrifying ugly undocumented miserable scary code (umh, that I wrote) by nice modern well-documented code.

comment:2 Changed 13 years ago by ncalexan

  • Summary changed from [with patch; needs review] rewrite load_session and save_session to use much better modern techniques (in particular merge the notebook and non-notebook implementations) to [with patch; with positive review] rewrite load_session and save_session to use much better modern techniques (in particular merge the notebook and non-notebook implementations)

I am not enough of a notebook user to test this thoroughly, but the code is good and the documentation is good. I say apply immediately!

One quibble: could the module comment make it clear why this is in Cython and not Python? I think there's a locals() trick at play, but it should be made clear none-the-less.

comment:3 Changed 13 years ago by mabshoff

Ok, I tracked down the issue for the reject of hunk two in worksheet.py. It expects:

if t.startswith('load_session'):
    filename = self.hunt_file(filename)

But the file contains:

filename = self.hunt_file(filename)

I am deleting hunk two as is and then apply the rest of the patch.

Cheers,

Michael

comment:4 Changed 13 years ago by mabshoff

  • Resolution set to fixed
  • Status changed from new to closed

Merged in Sage 3.0.alpha5

Note: See TracTickets for help on using tickets.