Helpful message if GAP fails to load a package

[ncohen]

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

[jferreira]

This patch works for me, applied to sage 5.8:

sage: sage.combinat.designs.block_design.!WittDesign(24)[[BR]]Incidence structure with 24 points and 759 blocks

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]

how does this ticket relate to #14039 ?

[dimpase]

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

[dimpase]

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

[ncohen]

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

[ncohen]

how does this ticket relate to #14039 ?

Oops...

Nathann

[ncohen]

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]

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.

[ncohen]

Oh, I see. Thanks :-)

Nathann

[jdemeyer]

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]

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

[ncohen]

Okayyyyyyyyyyyyyyyy....

[ncohen]

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

[jferreira]

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
**********************************************************************

[ncohen]

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

Nathann

[vbraun]

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?

[ncohen]

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]

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.

[ncohen]

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

[ncohen]

What about this ?

Nathann

[vbraun]

Fine with me.