Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#9554 closed defect (fixed)

Doctest failure in SageNB's sageinspect.py with #8988

Reported by: mpatel Owned by: jason, was
Priority: blocker Milestone: sage-4.5.2
Component: notebook Keywords:
Cc: davidloeffler, novoselt, vbraun, timdumol Merged in: sagenb-0.8.2
Authors: Leif Leonhardy Reviewers: Volker Braun
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description

This happens with the patches at #8988, which I've merged in the forthcoming 4.5.2.alpha0:

$ ./sage -t -long  local/lib/python2.6/site-packages/sagenb-0.8.1-py2.6.egg/sagenb/misc/sageinspect.py
sage -t -long "local/lib/python2.6/site-packages/sagenb-0.8.1-py2.6.egg/sagenb/misc/sageinspect.py"
**********************************************************************
File "/mnt/usb1/scratch/mpatel/tmp/sage-4.5.1-rm/local/lib/python2.6/site-packages/sagenb-0.8.1-py2.6.egg/sagenb/misc/sageinspect.py", line 997:
    sage: sage_getvariablename(A)
Expected:
    ['A', 'B']
Got:
    ['B', 'A']
**********************************************************************
1 items had failures:
   1 of   8 in __main__.example_22
***Test Failed*** 1 failures.

Attachments (1)

Change History (12)

comment:1 follow-up: Changed 11 years ago by vbraun

Can you double-check that you applied trac_8988-sageinspect_doctest_fix.patch from #8988? That patch was supposed to fix precisely this issue. In fact, there are 3 patches in #8988, see http://trac.sagemath.org/sage_trac/ticket/8988#comment:42

comment:2 Changed 11 years ago by mpatel

I've applied the 3 patches mentioned in #8988 comment 42. The failure above is in the notebook's sageinspect.py file, which is very similar to the Sage library's version.

Of course, we should try to reconcile the two files, to the extent it's permitted by SageNB's status as an independent project. I believe we've been updating them together when one changes, in order to keep both current.

comment:3 Changed 11 years ago by mpatel

I can make the SageNB patch a bit later this week and try to ensure it's merged into 4.5.2.alphaX.

comment:4 follow-up: Changed 11 years ago by leif

  • Status changed from new to needs_review

Please excuse if my upload violates any rules regarding Sage-SageNB collaboration...

The patch is almost the same as the one to the Sage library here: http://trac.sagemath.org/sage_trac/attachment/ticket/8988/trac_8988-sageinspect_doctest_fix.patch

I've built a SageNB 0.8.1.p1 spkg, installed it on 4.5.2.alpha0, and ./sage -t -long -sagenb passed all tests.

(I'm not going to upload the patched spkg though, it's almost 19MB. So if appropriate, someone else has to provide it on sage.math.)

-Leif

comment:5 follow-up: Changed 11 years ago by leif

P.S.:

leif@portland:~/Sage/spkgs/sagenb-0.8.1.p1/src/sagenb$ hg status
? sagenb/sagenb.po

(This is of course in vanilla 0.8.1, too.)

comment:6 in reply to: ↑ 1 ; follow-up: Changed 11 years ago by ddrake

Replying to vbraun:

Can you double-check that you applied trac_8988-sageinspect_doctest_fix.patch from #8988? That patch was supposed to fix precisely this issue. In fact, there are 3 patches in #8988, see http://trac.sagemath.org/sage_trac/ticket/8988#comment:42

All three patches were merged in 4.5.2.alpha0 -- changesets 14652-4.

comment:7 in reply to: ↑ 6 Changed 11 years ago by vbraun

  • Authors set to Leif Leonhardy
  • Cc Tim Dumol added
  • Reviewers set to Volker Braun
  • Status changed from needs_review to positive_review

Yes, as mpatel already pointed out, the problematic doctest is in sagenb/misc/sageinspect.py and not in sage/misc/sageinspect.py (sagenb vs. sage). The latter is fixed by the patch in #8988, and the former is fixed by leif's trac_9554-fix_order_of_var_names-SageNB-repo.patch. Both patches make identical changes to code that is duplicated in the notebook spkg.

Since the patch is clearly the right thing to do I'll give it a positive review. Whoever does the next SageNB release, please add this patch. The tracker bug for SageNB 0.8.2 is #9572 where this patch is already listed.

comment:8 Changed 11 years ago by vbraun

  • Cc timdumol added; Tim Dumol removed

comment:9 in reply to: ↑ 5 Changed 11 years ago by mpatel

Replying to leif:

P.S.:

leif@portland:~/Sage/spkgs/sagenb-0.8.1.p1/src/sagenb$ hg status
? sagenb/sagenb.po

(This is of course in vanilla 0.8.1, too.)

This is now #9580.

comment:10 Changed 11 years ago by mpatel

  • Merged in set to sagenb-0.8.2
  • Resolution set to fixed
  • Status changed from positive_review to closed

I've merged the patch into SageNB 0.8.2, which awaits review at #9572.

comment:11 in reply to: ↑ 4 Changed 11 years ago by mpatel

Replying to leif:

(I'm not going to upload the patched spkg though, it's almost 19MB. So if appropriate, someone else has to provide it on sage.math.)

Do you have a Sage cluster (i.e., sage.math, boxen.math, etc.) account? If not, William Stein can make one for you.

Note: See TracTickets for help on using tickets.