new style package for database_kohel

[vdelecroix]

Make a new style package for database_kohel

Upstream tarball available at

​http://www.labri.fr/perso/vdelecro/database_kohel-20160724.tar.gz

[vdelecroix]

(few minutes needed to upload the tarball)

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​e6902ed

Trac 21086: database_kohel new style package

[jdemeyer]

1. You need to check for errors in spkg-install (in particular, the mv commands).

2. You can replace

   if [ -d ${TARGET} ]
   then
   	echo "REMOVING PREVIOUS VERSION"
       rm -rf ${TARGET}
   fi
   

by

rm -rf ${TARGET}

3. I don't like this error handling:

            try:
                coeff_list = _dbz_to_integer_list(modpol)
           except IOError:
               raise KeyError("No database entry for modular polynomial of level %s"%level)
   

I think it would be better to raise the error directly in _dbz_to_integer_list and then only if the file really cannot be opened. If the error is raised while reading the file, it should be propagated as IOError. I also think that KeyError is not really appropriate, since it's not a key. I would prefer LookupError.

[jdemeyer]

I should also add that this package is not as important as it used to be, since PARI can now compute the modular polynomials provided by this package.

[jdemeyer]

For the dependencies, can you use a more explicit # no dependencies?

[vdelecroix]

Replying to jdemeyer:

I should also add that this package is not as important as it used to be, since PARI can now compute the modular polynomials provided by this package.

Could you provide the commands available to check the data in the package?

What do you think about using pari and deprecate the database?

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​04e9d9d

Trac 21086: review

[jdemeyer]

Replying to vdelecroix:

Could you provide the commands available to check the data in the package?

Maybe you should ask the PARI developers (are you currently in Bordeaux?)

What do you think about using pari

This already happens. The database is no longer used by anything else in Sage.

and deprecate the database?

I don't think this is needed. It is still useful to avoid long computations if you need the large polynomials.

[jdemeyer]

You didn't fix this:

1. You need to check for errors in spkg-install (in particular, the mv commands).

[jdemeyer]

Another detail (feel free to fix this or not): I don't think that the extra arguments to __getitem__ are useful:

def __getitem__(self, disc, level=1, var='x'):

I would change this to

def __getitem__(self, disc):

[git]

Branch pushed to git repo; I updated commit sha1. New commits:

​aa84574	Trac 21086: check for errors in spkg-install
​7df7bce	Trac 21086: remove keyword argument to __getitem__