Helpful message if GAP fails to load a package #14483

nathanncohen · 2013-04-24T15:54:43Z

Very short patch ! I met that problem while trying to run

sage: sage.combinat.designs.block_design.WittDesign(24)

and I had no idea how to install a gap package in the copy of gap that Sage contains...

Nathann

CC: @dimpase @vbraun @malb

Component: combinatorics

Author: Nathann Cohen

Reviewer: Dmitrii Pasechnik

Merged: sage-5.10.beta1

Issue created by migration from https://trac.sagemath.org/ticket/14483

The text was updated successfully, but these errors were encountered:

jeffpferreira · 2013-04-25T00:49:59Z

comment:2

This patch works for me, applied to sage 5.8:

sage: sage.combinat.designs.block_design.WittDesign(24)

Incidence structure with 24 points and 759 block

However, after being prompted to run install_package("gap_packages"), gap_packages-4.5.7 is found. After you updated the spkg was I supposed to find gap_packages-4.6.3 ?

dimpase · 2013-04-25T04:31:17Z

comment:3

how does this ticket relate to #14039 ?

dimpase · 2013-04-25T04:37:54Z

comment:4

removed http://www.steinertriples.fr/gap-4.6.3.p0.spkg from the ticket description.

dimpase · 2013-04-25T04:39:23Z

comment:5

Positive review - the patch works on Sage 4.5.9.beta5 just fine. GAP updates should be dealt with elsewhere.

jdemeyer · 2013-04-25T05:21:00Z

Reviewer: Dmitrii Pasechnik

nathanncohen · 2013-04-25T07:25:20Z

comment:8

However, after being prompted to run install_package("gap_packages"), gap_packages-4.5.7 is found. After you updated the spkg was I supposed to find gap_packages-4.6.3 ?

Nononono, gap and gap_packages are different spkg. Besides, when you ask Sage to install a package for you it will download the latest "accepted" package, so not the one from this ticket :-)

Nathann

nathanncohen · 2013-04-25T07:25:42Z

comment:9

how does this ticket relate to #14039 ?

Oops...

Nathann

nathanncohen · 2013-04-25T07:26:08Z

comment:10

Positive review - the patch works on Sage 4.5.9.beta5 just fine.

Thaaaaaaanks !

GAP updates should be dealt with elsewhere.

Yepyep. Sorry 'bout that :-)

Nathann

vbraun · 2013-04-25T09:28:27Z

comment:11

Doesn't really matter here, but in general error messages should be kept brief. If you want to show the user some explanation or hints how to get around a potential pitfall you should use warnings so that it is only printed once during the session.

nathanncohen · 2013-04-25T09:30:06Z

comment:12

Oh, I see. Thanks :-)

Nathann

jdemeyer · 2013-04-25T12:53:34Z

comment:13

sage -t devel/sage/sage/groups/perm_gps/permgroup.py
**********************************************************************
File "devel/sage/sage/groups/perm_gps/permgroup.py", line 152, in sage.groups.perm_gps.permgroup.load_hap
Failed example:
    sage.groups.perm_gps.permgroup.load_hap()
Exception raised:
    Traceback (most recent call last):
      File "/mazur/release/merger/sage-5.10.beta1/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 466, in _run
        self.execute(example, compiled, test.globs)
      File "/mazur/release/merger/sage-5.10.beta1/local/lib/python2.7/site-packages/sage/doctest/forker.py", line 825, in execute
        exec compiled in globs
      File "<doctest sage.groups.perm_gps.permgroup.load_hap[0]>", line 1, in <module>
        sage.groups.perm_gps.permgroup.load_hap()
      File "/mazur/release/merger/sage-5.10.beta1/local/lib/python2.7/site-packages/sage/groups/perm_gps/permgroup.py", line 157, in load_hap
        gap.load_package("hap")
      File "/mazur/release/merger/sage-5.10.beta1/local/lib/python2.7/site-packages/sage/interfaces/gap.py", line 514, in load_package
        "install_package(\"gap_packages\")"%pkg)
    RuntimeError: Error loading Gap package hap. You may want to install a collection of GAP packages by installing the 'gap_packages' Sage spkg. If so, then run instal
l_package("gap_packages")
**********************************************************************

jdemeyer · 2013-04-25T12:53:54Z

comment:14

I agree with Volker that a long warning is better than a long error message.

nathanncohen · 2013-04-25T15:39:36Z

comment:15

Okayyyyyyyyyyyyyyyy....

nathanncohen · 2013-04-25T15:54:26Z

comment:16

What do you think of this ? Honestly I preferred the previous version... A warning just before an exception does not stand out at all ... :-/

Nathann

jeffpferreira · 2013-04-28T02:07:06Z

comment:18

sage -t devel/sage/sage/groups/perm_gps/permgroup.py    
**********************************************************************
File "/Applications/sage/devel/sage/sage/groups/perm_gps/permgroup.py", line 152:
    sage: sage.groups.perm_gps.permgroup.load_hap()
Exception raised:
    Traceback (most recent call last):
      File "/Applications/sage/local/bin/ncadoctest.py", line 1231, in run_one_test
        self.run_one_example(test, example, filename, compileflags)
      File "/Applications/sage/local/bin/sagedoctest.py", line 38, in run_one_example
        OrigDocTestRunner.run_one_example(self, test, example, filename, compileflags)
      File "/Applications/sage/local/bin/ncadoctest.py", line 1172, in run_one_example
        compileflags, 1) in test.globs
      File "<doctest __main__.example_1[2]>", line 1, in <module>
        sage.groups.perm_gps.permgroup.load_hap()###line 152:
    sage: sage.groups.perm_gps.permgroup.load_hap()
      File "/Applications/sage/local/lib/python/site-packages/sage/groups/perm_gps/permgroup.py", line 157, in load_hap
        gap.load_package("hap")
      File "/Applications/sage/local/lib/python/site-packages/sage/interfaces/gap.py", line 511, in load_package
        raise RuntimeError('Error loading Gap package %s'%pkg)
    RuntimeError: Error loading Gap package hap
**********************************************************************

nathanncohen · 2013-04-28T08:10:54Z

comment:19

Fixed ! What do you think of this warning/exception thing, by the way ?

Nathann

vbraun · 2013-04-28T10:40:50Z

comment:20

I'm pretty sure the doctest will fail if you happen to have gap_packages installed (which includes HAP). How about we use gap.load_package("foobar") or something like that instead?

nathanncohen · 2013-04-28T10:45:51Z

comment:21

Such a doctest already exists in interfaces/gap.py. Do you think that we should remove this doctest from permgroup.py ? I think it make sense.

What do you think of this warning/exception thing ?

Nathann

vbraun · 2013-04-28T11:22:36Z

comment:22

The doctest for the load_package() isn't necessary if it is tested elsewhere, but it is the natural place in the docstring to point out that you have to install HAP. I'll let you decide if you want to keep it or not.

As I said already, helpful explanations should be warnings not errors. The error message should be less than a whole line if possible. Advanced users shouldn't be drowned in paragraphs of text for simple errors. We could make warnings more prominent (say, using colors or bold text), but thats for another ticket.

nathanncohen · 2013-04-28T11:27:24Z

comment:23

Then I will probaly remove the doctest.

My problem with exceptions and warnings is not so general : in that case, a warning is printed just before the exception is raised, and no one sees the warning as a result. It would make more sense to just raise an exception. I will try to make the error message shorter.

Nathann

nathanncohen · 2013-04-28T13:41:49Z

comment:24

Attachment: trac_14483.patch.gz

What about this ?

Nathann

vbraun · 2013-04-28T14:22:49Z

comment:25

Fine with me.

jdemeyer · 2013-04-30T09:40:18Z

Merged: sage-5.10.beta1

nathanncohen mannequin added this to the sage-5.10 milestone Apr 24, 2013

nathanncohen mannequin added c: packages: standard labels Apr 24, 2013

nathanncohen mannequin assigned jdemeyer Apr 24, 2013

nathanncohen mannequin added the s: needs review label Apr 24, 2013

dimpase added s: needs info and removed s: needs review labels Apr 25, 2013

This comment has been minimized.

Sign in to view

dimpase added s: needs review and removed s: needs info labels Apr 25, 2013

dimpase added s: positive review and removed s: needs review labels Apr 25, 2013

jdemeyer changed the title ~~Update of GAP, and a an helpful message it fails at loading a package~~ Helpful message if GAP fails to load a package Apr 25, 2013

jdemeyer removed the s: positive review label Apr 25, 2013

jdemeyer added the s: needs work label Apr 25, 2013

nathanncohen mannequin added s: needs review and removed s: needs work labels Apr 25, 2013

jeffpferreira added s: needs work and removed s: needs review labels Apr 28, 2013

nathanncohen mannequin added s: needs review and removed s: needs work labels Apr 28, 2013

vbraun added s: positive review and removed s: needs review labels Apr 28, 2013

jdemeyer removed the s: positive review label Apr 30, 2013

jdemeyer closed this as completed Apr 30, 2013

nathanncohen mannequin mentioned this issue May 8, 2013

Some cleaup in sage/combinat/design #14499

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Helpful message if GAP fails to load a package #14483

Helpful message if GAP fails to load a package #14483

nathanncohen mannequin commented Apr 24, 2013

jeffpferreira commented Apr 25, 2013

dimpase commented Apr 25, 2013

This comment has been minimized.

dimpase commented Apr 25, 2013

dimpase commented Apr 25, 2013

jdemeyer commented Apr 25, 2013

nathanncohen mannequin commented Apr 25, 2013

nathanncohen mannequin commented Apr 25, 2013

nathanncohen mannequin commented Apr 25, 2013

vbraun commented Apr 25, 2013

nathanncohen mannequin commented Apr 25, 2013

jdemeyer commented Apr 25, 2013

jdemeyer commented Apr 25, 2013

nathanncohen mannequin commented Apr 25, 2013

nathanncohen mannequin commented Apr 25, 2013

jeffpferreira commented Apr 28, 2013

nathanncohen mannequin commented Apr 28, 2013

vbraun commented Apr 28, 2013

nathanncohen mannequin commented Apr 28, 2013

vbraun commented Apr 28, 2013

nathanncohen mannequin commented Apr 28, 2013

nathanncohen mannequin commented Apr 28, 2013

vbraun commented Apr 28, 2013

jdemeyer commented Apr 30, 2013

Helpful message if GAP fails to load a package #14483

Helpful message if GAP fails to load a package #14483

Comments

nathanncohen mannequin commented Apr 24, 2013

jeffpferreira commented Apr 25, 2013

dimpase commented Apr 25, 2013

This comment has been minimized.

dimpase commented Apr 25, 2013

dimpase commented Apr 25, 2013

jdemeyer commented Apr 25, 2013

nathanncohen mannequin commented Apr 25, 2013

nathanncohen mannequin commented Apr 25, 2013

nathanncohen mannequin commented Apr 25, 2013

vbraun commented Apr 25, 2013

nathanncohen mannequin commented Apr 25, 2013

jdemeyer commented Apr 25, 2013

jdemeyer commented Apr 25, 2013

nathanncohen mannequin commented Apr 25, 2013

nathanncohen mannequin commented Apr 25, 2013

jeffpferreira commented Apr 28, 2013

nathanncohen mannequin commented Apr 28, 2013

vbraun commented Apr 28, 2013

nathanncohen mannequin commented Apr 28, 2013

vbraun commented Apr 28, 2013

nathanncohen mannequin commented Apr 28, 2013

nathanncohen mannequin commented Apr 28, 2013

vbraun commented Apr 28, 2013

jdemeyer commented Apr 30, 2013