Opened 7 years ago

Closed 7 years ago

#14344 closed defect (fixed)

Fix packages test

Reported by: kcrisman Owned by: mvngu
Priority: blocker Milestone: sage-5.9
Component: doctest coverage Keywords:
Cc: kini, jhpalmieri Merged in: sage-5.9.beta2
Authors: Keshav Kini Reviewers: Karl-Dieter Crisman
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:


Here is a request from a sage-devel thread:

The following test fails because I have some optional packages installed - 

$ sage -t  --long -force_lib devel/sage/sage/misc/ 
sage -t --long -force_lib "devel/sage/sage/misc/" 
File "/home/rajeev/software/general/sage/devel/sage/sage/misc/", 
line 119: 
    sage: install_package() 
    ['atlas...', 'blas...', ...] 
    ['PyQt_x11-4.9.1.p0', 'atlas-3.8.4.p1', 'blas-20070724', 
'boehm_gc-7.2.alpha6.p2', 'boost-cropped-1.34.1', 

The consensus seemed to be simply adding more ellipses.

> Just add periods *before* 'atlas...' as well. 

And after it (between 'atlas...' and 'blas...'), just in case. Maybe something like

        ['atlas...', ..., 'python...', ...]
This is a better idea, IMO. 

Attachments (1)

trac_14344-installed-packages.patch (578 bytes) - added by kini 7 years ago.
apply to $SAGE_ROOT/devel/sage

Download all attachments as: .zip

Change History (10)

comment:1 Changed 7 years ago by kini

Here's a patch.

comment:2 Changed 7 years ago by kcrisman

This patch isn't compatible with #10508, and goodness knows that ticket doesn't need any more dependencies... is it okay to ask to rebase to a "needs work" ticket? Maybe the version with python as John suggested? Or the filter version you mentioned in the relevant thread, if you think that one is better (though perhaps it's harder to understand what it's doing).

Changed 7 years ago by kini

apply to $SAGE_ROOT/devel/sage

comment:3 Changed 7 years ago by kini

How's this patch?

The filter thing I posted on sage-devel is semantically nicer, IMO, but I agree it looks pretty ugly and hard to read in Python, and this test isn't exactly run in a tight loop so performance is sort of irrelevant :)

comment:4 Changed 7 years ago by kcrisman

I just realized that probably they would still need to rebase at #10508, so I may have wasted your time... anyway, looks good. I had some trouble finding an spkg that would actually compile on my box that was before atlas, though, to make sure the test worked! The one in the thread needs qmake, and the ace package is pretty outdated, apparently, and seems to remove some stuff it maybe shouldn't. Anyway, 4ti2 worked and the test still passes, so we're fine.

comment:5 Changed 7 years ago by kcrisman

  • Authors set to Keshav Kini
  • Reviewers set to Karl-Dieter Crisman
  • Status changed from new to needs_review

comment:6 Changed 7 years ago by kcrisman

  • Status changed from needs_review to positive_review

comment:7 Changed 7 years ago by kini

This is such a tiny patch, I really couldn't complain about any time being wasted :) Since you've given this positive review, I'll go rebase the patch at #10508 on this.

comment:8 Changed 7 years ago by jdemeyer

  • Priority changed from minor to blocker

comment:9 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.9.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.