Raise exceptions when database_gap is not installed

[ncohen]

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.

[ncohen]

New commits:

​2dbb38d

trac #16702: Raise exceptions when database_gap is not installed

[dimpase]

This should be implemented using try, I think.

[ncohen]

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

[dimpase]

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

[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.

There are ways to extend GAP and/or Sage in place not involving installing Sage packages...

I don't understand that.

Nathann

[dimpase]

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.

[git]

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

[dimpase]

LGTM