Opened 6 years ago
Closed 6 years ago
#14483 closed defect (fixed)
Helpful message if GAP fails to load a package
Reported by: | ncohen | Owned by: | jdemeyer |
---|---|---|---|
Priority: | major | Milestone: | sage-5.10 |
Component: | combinatorics | Keywords: | |
Cc: | dimpase, vbraun, malb | Merged in: | sage-5.10.beta1 |
Authors: | Nathann Cohen | Reviewers: | Dmitrii Pasechnik |
Report Upstream: | N/A | Work issues: | |
Branch: | Commit: | ||
Dependencies: | Stopgaps: |
Description (last modified by )
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
Attachments (1)
Change History (27)
comment:1 Changed 6 years ago by
- Status changed from new to needs_review
comment:2 follow-up: ↓ 8 Changed 6 years ago by
comment:3 follow-up: ↓ 9 Changed 6 years ago by
- Status changed from needs_review to needs_info
how does this ticket relate to #14039 ?
comment:4 Changed 6 years ago by
- Description modified (diff)
- Status changed from needs_info to needs_review
removed http://www.steinertriples.fr/gap-4.6.3.p0.spkg from the ticket description.
comment:5 follow-up: ↓ 10 Changed 6 years ago by
- Status changed from needs_review to positive_review
Positive review - the patch works on Sage 4.5.9.beta5 just fine. GAP updates should be dealt with elsewhere.
comment:6 Changed 6 years ago by
- Component changed from packages: standard to combinatorics
- Type changed from enhancement to defect
comment:7 Changed 6 years ago by
- Reviewers set to Dmitrii Pasechnik
- Summary changed from Update of GAP, and a an helpful message it fails at loading a package to Helpful message if GAP fails to load a package
comment:8 in reply to: ↑ 2 Changed 6 years ago by
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
comment:9 in reply to: ↑ 3 Changed 6 years ago by
comment:10 in reply to: ↑ 5 Changed 6 years ago by
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
comment:11 Changed 6 years ago by
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.
comment:12 Changed 6 years ago by
Oh, I see. Thanks :-)
Nathann
comment:13 Changed 6 years ago by
- Status changed from positive_review to needs_work
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") **********************************************************************
comment:14 Changed 6 years ago by
I agree with Volker that a long warning is better than a long error message.
comment:15 Changed 6 years ago by
Okayyyyyyyyyyyyyyyy....
comment:16 Changed 6 years ago by
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
comment:17 Changed 6 years ago by
- Status changed from needs_work to needs_review
comment:18 Changed 6 years ago by
- Status changed from needs_review to needs_work
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 **********************************************************************
comment:19 Changed 6 years ago by
- Status changed from needs_work to needs_review
Fixed ! What do you think of this warning/exception thing, by the way ?
Nathann
comment:20 Changed 6 years ago by
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?
comment:21 Changed 6 years ago by
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
comment:22 Changed 6 years ago by
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.
comment:23 Changed 6 years ago by
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
Changed 6 years ago by
comment:24 Changed 6 years ago by
What about this ?
Nathann
comment:26 Changed 6 years ago by
- Merged in set to sage-5.10.beta1
- Resolution set to fixed
- Status changed from positive_review to closed
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 ?