Opened 13 years ago

Closed 13 years ago

#7650 closed defect (fixed)

Fix sagenb doctesting

Reported by: Mitesh Patel Owned by: William Stein
Priority: blocker Milestone: sage-4.3.1
Component: notebook Keywords:
Cc: William Stein, Tim Dumol, Mike Hansen, Jason Grout Merged in: sage-4.3.1.rc1
Authors: Mitesh Patel Reviewers: Tim Dumol
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by Mitesh Patel)

We have not been doctesting SageNB files as often as we test Sage files.

Please apply

  • trac_7650-scripts_doctest_force_lib_v5.patch to the scripts repository. This adds two options to sage -t: -sagenb tests all SageNB files and -force_lib tests files as if they live in SAGE_ROOT/devel/sage, i.e., in the Sage library.

and replace SAGE_ROOT/makefile with

  • makefile. This makes make test and make ptest test all SageNB files, too.

Dependencies: Sage 4.3.1.alpha1, #7269.

Attachments (14)

trac_7650-sagenb_doctesting.patch (223.1 KB) - added by Mitesh Patel 13 years ago.
Add --force_lib option to sage-doctest. Use os.path.join. scripts repo.
trac_7650-scripts_doctest_force_lib.patch (8.7 KB) - added by Mitesh Patel 13 years ago.
Fix sagenb doctests. sagenb repo.
trac_7650-scripts_doctest_force_lib_v2.patch (8.7 KB) - added by Mitesh Patel 13 years ago.
Updated scripts repo patch. Replaces previous.
trac_7650-sagenb_doctesting_v2.patch (224.9 KB) - added by Mitesh Patel 13 years ago.
Quit spawned worksheet processes. Replaces previous sagenb repo patch.
trac_7650-scripts_doctest_force_lib_v3.patch (10.0 KB) - added by Mitesh Patel 13 years ago.
Auto-detect site-packages. scripts repo. Replaces previous.
trac_7650-scripts_doctest_force_lib_v4.patch (12.1 KB) - added by Mitesh Patel 13 years ago.
Adds -sagenb option. scripts repo. Replaces previous.
makefile (2.6 KB) - added by Mitesh Patel 13 years ago.
Updated SAGE_ROOT/makefile.
trac_7650-scripts_doctest_force_lib_v5.patch (16.7 KB) - added by Mitesh Patel 13 years ago.
Document new options. More os.path.join-ery. Replaces previous.
trac_7650-sagenb_doctesting_v3.patch (224.5 KB) - added by Mitesh Patel 13 years ago.
Rebased vs. #7482's v4. Replaces previous sagenb patch.
trac_7650-sagenb_doctesting_v4.patch (222.3 KB) - added by Mitesh Patel 13 years ago.
Rebased vs. 4.3.1.alpha0 + #7269. Replaces previous sagenb patch.
trac_7650-sagenb_doctesting_v5.patch (221.9 KB) - added by Mitesh Patel 13 years ago.
Fix tags / user_view asymmetry. Replaces previous sagenb patch.
trac_7650-sagenb_doctesting_v6.patch (223.0 KB) - added by Mitesh Patel 13 years ago.
Fix interact doctests for 4.3.1.alpha2 (colors.py). Replaces previous sagenb patch.
trac_7650-reviewer.patch (10.8 KB) - added by Tim Dumol 13 years ago.
A few documentation fixes (precision errors and style), and a couple of changes to try-except blocks to make it easier to transition to Python 3
trac_7650-reviewer_v2.patch (11.5 KB) - added by Mitesh Patel 13 years ago.
Fix two doctest failures. Replaces reviewer patch. sagenb repo.

Download all attachments as: .zip

Change History (37)

comment:1 Changed 13 years ago by Mitesh Patel

Part of the problem is that notebook directories now need to end in .sagenb.

I'm working on a patch for this and whatever other problems I can fix.

comment:2 Changed 13 years ago by Mitesh Patel

sage -t -verbose "notebook.py"                              
Traceback (most recent call last):
  File "/home/.sage//tmp/.doctest_notebook.py", line 18, in <module>
    from notebook import *
  File "/home/.sage/tmp/notebook.py", line 38, in <module>
    import css          # style
ImportError: No module named css
         [2.5 s]
exit code: 1024
 
----------------------------------------------------------------------
The following tests failed:


        sage -t -verbose "notebook.py"
Total time for all tests: 2.5 seconds

Adding

import sys
sys.path.append('/home/sage/notebook/sagenb/sagenb')
sys.path.append('/home/sage/notebook/sagenb/sagenb/notebook')

to the top of notebook.py allows testing to proceed. I'm not sure about a real solution.

Changed 13 years ago by Mitesh Patel

Add --force_lib option to sage-doctest. Use os.path.join. scripts repo.

Changed 13 years ago by Mitesh Patel

Fix sagenb doctests. sagenb repo.

comment:3 Changed 13 years ago by Mitesh Patel

Authors: Mitesh Patel
Priority: majorblocker
Status: newneeds_review

Please note: The attachments (or their descriptions) are switched. I apologize for this.

Anyway, with the scripts repository patch, we can do, e.g.,

$ sage -t --force_lib sagenb/

With the sagenb repository patch applied to #7625 + #7483 + #7482 + #7514 + #7648, all tests but 3 pass. The failures are in sagenb/misc/sageinspect.py (cf. #7514). The Selenium tests still pass.

Remarks:

  • I hope I did not create false-negatives.
  • This sage-notebook thread might be relevant to some tests that evaluate cells.
  • I "untabified" cell.py, worksheet.py, notebook.py, and twist.py.
  • I added 'nodoctest' to simple/twist.py, since at least one test appears to hang indefinitely. I don't know why.
  • Feel free to lower the priority.

comment:4 Changed 13 years ago by Mitesh Patel

Cc: Mike Hansen added

comment:5 Changed 13 years ago by Mitesh Patel

Should we add, e.g.,

sage -t --force_lib $SAGE_ROOT/local/lib/python/site-packages/sagenb

to $SAGE_ROOT/makefile? Or sage-test?

comment:6 Changed 13 years ago by William Stein

Status: needs_reviewneeds_work

I like this patch a lot. However, a nitpick: get rid of the / before tmp here:

SAGE_TESTDIR = os.path.join(DOT_SAGE, "/tmp") 

Likewise, elsewhere in the patch there are a bunch of os.path.joins combined with explicit* slashes.

Regarding adding sage -t sagenb, etc., to sage-test or whatever: "yes!"

Changed 13 years ago by Mitesh Patel

Updated scripts repo patch. Replaces previous.

Changed 13 years ago by Mitesh Patel

Quit spawned worksheet processes. Replaces previous sagenb repo patch.

comment:7 Changed 13 years ago by William Stein

Milestone: sage-4.3.1sage-4.3

Changed 13 years ago by Mitesh Patel

Auto-detect site-packages. scripts repo. Replaces previous.

Changed 13 years ago by Mitesh Patel

Adds -sagenb option. scripts repo. Replaces previous.

Changed 13 years ago by Mitesh Patel

Attachment: makefile added

Updated SAGE_ROOT/makefile.

comment:8 Changed 13 years ago by Mitesh Patel

Status: needs_workneeds_review

With trac_7650-scripts_doctest_force_lib_v4.patch and an updated top-level makefile, we can do

  • make test
  • make ptest
  • sage -t -sagenb
  • sage -tp -sagenb

etc. Maybe we should unify the test scripts and use optparse?

comment:9 Changed 13 years ago by Tim Dumol

Status: needs_reviewneeds_work

The new options to sage -t (-force_lib and -sagenb) are not documented in the help message. Please do so.

comment:10 Changed 13 years ago by William Stein

Milestone: sage-4.3sage-4.3.1

I'm declaring a total feature freeze on sage-4.3.

comment:11 Changed 13 years ago by Mitesh Patel

Also: Be more agressive with os.path.join-ery.

Changed 13 years ago by Mitesh Patel

Document new options. More os.path.join-ery. Replaces previous.

comment:12 Changed 13 years ago by Mitesh Patel

Description: modified (diff)
Status: needs_workneeds_review

Changed 13 years ago by Mitesh Patel

Rebased vs. #7482's v4. Replaces previous sagenb patch.

comment:13 Changed 13 years ago by Mitesh Patel

Description: modified (diff)

Changed 13 years ago by Mitesh Patel

Rebased vs. 4.3.1.alpha0 + #7269. Replaces previous sagenb patch.

comment:14 Changed 13 years ago by Mitesh Patel

Description: modified (diff)

comment:15 Changed 13 years ago by Mitesh Patel

I've attached a rebased sagenb patch. I added """nodoctests""" to the top of sagenb.misc.ipaddr. It's also easy to make its doctest pass, but I haven't done this. Please let me know, if I should.

Changed 13 years ago by Mitesh Patel

Fix tags / user_view asymmetry. Replaces previous sagenb patch.

comment:16 Changed 13 years ago by Mitesh Patel

Description: modified (diff)

comment:17 Changed 13 years ago by Mitesh Patel

V5 makes it so a worksheet reconstructed from the basic representation of another has the same tags and user_view as the original.

Changed 13 years ago by Mitesh Patel

Fix interact doctests for 4.3.1.alpha2 (colors.py). Replaces previous sagenb patch.

comment:18 in reply to:  17 Changed 13 years ago by Tim Dumol

Replying to mpatel:

V5 makes it so a worksheet reconstructed from the basic representation of another has the same tags and user_view as the original.

Shouldn't this be put in a new ticket?

comment:19 Changed 13 years ago by Mitesh Patel

Yes, but it's already here. :) In the course of fixing many failed doctests, I noticed it and a few other small problems:

  • In run_notebook.notebook_twisted: Replaced nb.directory() (not defined) with nb._dir.
  • In worksheet: Replaced self.__collaborators (AttributeError) with self.collaborators() in a few places.
  • In worksheet.set_filename:
    -        self.__dir = os.path.join(self.notebook().worksheet_directory(), filename)
    +        self.__dir = os.path.join(self.notebook()._dir, filename)
    
  • In worksheet.tags:
                 d = dict(self.__user_view)
             except AttributeError:
                 self.user_view(self.owner())
    -            d = self.__user_view
    +            d = copy.copy(self.__user_view)
             for user, val in d.iteritems():
    -            d[user] = [val]
    +            if not isinstance(val, list):
    +                d[user] = [val]
             return d
    
    This ensures the tests in Worksheet.reconstruct_from_basic pass and that calling tags does not alter the current user view, e.g., turning 1 into [1].

Also:

  • Removed argument verbose (obsolete) from Cell.set_introspect_html.

comment:20 Changed 13 years ago by Tim Dumol

Cc: Jason Grout added

This bags a positive review from me, except for the few changes I've made in the reviewer patch. Can someone sign them off?

Changed 13 years ago by Tim Dumol

Attachment: trac_7650-reviewer.patch added

A few documentation fixes (precision errors and style), and a couple of changes to try-except blocks to make it easier to transition to Python 3

Changed 13 years ago by Mitesh Patel

Attachment: trac_7650-reviewer_v2.patch added

Fix two doctest failures. Replaces reviewer patch. sagenb repo.

comment:21 Changed 13 years ago by Mitesh Patel

Description: modified (diff)
Reviewers: Tim Dumol
Status: needs_reviewpositive_review

V2 of the reviewer patch:

  • Adds a missing dot (.) in interact.py.
  • Moves # output depends on timezone to the input statement in worksheet.py.
  • [Pre-existing] most be defined --> must be defined.

comment:22 Changed 13 years ago by Robert Miller

Merged in: sage-4.3.1.rc1

I applied the scripts patch and replaced makefile in SAGE_ROOT. Please post a sagenb-0.5.1.spkg.

comment:23 Changed 13 years ago by Robert Miller

Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.