Opened 7 years ago
Closed 7 years ago
#16702 closed enhancement (fixed)
Raise exceptions when database_gap is not installed
Reported by: | ncohen | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-6.3 |
Component: | group theory | Keywords: | |
Cc: | dimpase | Merged in: | |
Authors: | Nathann Cohen | Reviewers: | Dima Pasechnik |
Report Upstream: | N/A | Work issues: | |
Branch: | 758dd75 (Commits, GitHub, GitLab) | Commit: | 758dd7577528561d33e13e1a8f5b29d07d72f853 |
Dependencies: | Stopgaps: |
Description (last modified by )
Before
sage: graphs.ChvatalGraph().automorphism_group().structure_description() ... Error, the Small Groups identification is required but not installed executing StructureDescription($sage1);
After
sage: graphs.ChvatalGraph().automorphism_group().structure_description() ... RuntimeError: You must install the optional database_gap package first.
Change History (10)
comment:1 Changed 7 years ago by
- Branch set to u/ncohen/16702
- Cc dimpase added
- Commit set to 2dbb38d2d5d528971fde504390f903d42969f6ff
- Description modified (diff)
- Status changed from new to needs_review
comment:2 Changed 7 years ago by
This should be implemented using try
, I think.
comment:3 follow-up: ↓ 4 Changed 7 years ago by
I copy/pasted the test which is already used for gap_packages. Plus you don't have any way to differentiate exceptions originating from a gap code I guess. If you try/catch you could catch too much and say that the package is not installed when something else is going wrong.
Nathann
comment:4 in reply to: ↑ 3 ; follow-up: ↓ 5 Changed 7 years ago by
Replying to ncohen:
I copy/pasted the test which is already used for gap_packages.
have you ever noticed that Sage is full of crappy code, in some places at least ? ;-)
Plus you don't have any way to differentiate exceptions originating from a gap code I guess. If you try/catch you could catch too much and say that the package is not installed when something else is going wrong.
I am not saying that inside the exception processing block you should not test for installation of this package.
But without try
your patch is too intrusive.
There are ways to extend GAP and/or Sage in place not involving installing Sage packages...
comment:5 in reply to: ↑ 4 ; follow-up: ↓ 6 Changed 7 years ago by
I am not saying that inside the exception processing block you should not test for installation of this package. But without
try
your patch is too intrusive.
I don't see why, and I don't see what you would write instead.
There are ways to extend GAP and/or Sage in place not involving installing Sage packages...
I don't understand that.
Nathann
comment:6 in reply to: ↑ 5 Changed 7 years ago by
Replying to ncohen:
I am not saying that inside the exception processing block you should not test for installation of this package. But without
try
your patch is too intrusive.I don't see why, and I don't see what you would write instead.
e.g.
def group_id(self): try: return [Integer(n) for n in self._gap_().IdGroup()] except: if not is_package_installed('database_gap'): raise RuntimeError("You must install the optional database_gap package first.") print "oops..." raise
There are ways to extend GAP and/or Sage in place not involving installing Sage packages...
I don't understand that.
often it's just copying some files into $SAGE_LOCAL/gap
that does the trick.
That is, installing GAP packages (see http://gap-system.org/Packages/packages.html) often basically amounts to this.
comment:7 Changed 7 years ago by
- Commit changed from 2dbb38d2d5d528971fde504390f903d42969f6ff to 758dd7577528561d33e13e1a8f5b29d07d72f853
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
758dd75 | trac #16702: Raise exceptions when database_gap is not installed
|
comment:8 Changed 7 years ago by
- Reviewers set to Dima Pasechnik
comment:10 Changed 7 years ago by
- Branch changed from u/ncohen/16702 to 758dd7577528561d33e13e1a8f5b29d07d72f853
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
trac #16702: Raise exceptions when database_gap is not installed