remove sqlalchemy

[ohanar]

It is not used in sage nor sagenb and can easily be installed with pip.

CC: @jdemeyer

Component: packages: standard

Author: R. Andrew Ohana, Jeroen Demeyer

Branch/Commit: 7b582a5

Reviewer: Vincent Delecroix, Karl-Dieter Crisman, Thierry Monteil

Issue created by migration from https://trac.sagemath.org/ticket/15593

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from b94fd5a to d701c15

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

d701c15

Merge branch 'trac/develop' into optionalize

[jdemeyer]

comment:4

I dislike

fn = open(filename, 'wb') # check filename
fn.close()

cdef FILE *out = fopen(filename, 'wb')

gdImagePng(im, out)

fclose(out)

You can open the file using Python and access its FILE* with PyFile_AsFile.

[ohanar]

comment:5

Replying to @jdemeyer:

I dislike ...

Sure, I didn't really rethink any of the code, just refactored what was already there a bit.

[ohanar]

comment:6

I don't want to use PyFile_AsFile though since it isn't python 3 compatible.

[simon-king-jena]

comment:7

Since I need sqlalchemy (or at least some database), I'll not review it. Just some questions: Is there the plan to turn the current standard sqlalchemy spkg into an optional one? Will the current standard spkg (version 0.5.8) be upgraded when being made an optional spkg (version 0.9.3)?

[ohanar]

comment:8

Replying to @simon-king-jena:

Since I need sqlalchemy (or at least some database), I'll not review it. Just some questions: Is there the plan to turn the current standard sqlalchemy spkg into an optional one? Will the current standard spkg (version 0.5.8) be upgraded when being made an optional spkg (version 0.9.3)?

I've not removed sqlalchemy from the repository, so it will remain as an optional spkg (although not upgraded, probably a trivial upgrade though). sqlalchemy is not a database, but rather a wrapper for sql databases to give them a unified interface. We currently ship sqlite as our database of choice.

[simon-king-jena]

comment:9

Remark on sqlalchemy: As noted on sage-devel, the current spkg-install of sqlalchemy contains a bug. It says

rm -rf "$SAGE_LOCAL/lib/python/site-packages/SQLAlchemy*"

but should say

rm -rf "$SAGE_LOCAL"/lib/python/site-packages/SQLAlchemy*

as otherwise the old sqlalchemy version will not be removed and will still be used even when installing a new version.

Otherwise, upgrading sqlalchemy-spkg is trivial. Shall I open a separate ticket?

[jasongrout]

comment:10

If I understand the sqlalchemy spkg files correctly, this spkg-install does nothing more than a standard "python setup.py install". Over the years, there have been a fair number of such packages, or at least requests for such packages. How about we make "sage -i" try to "pip install" a package if it can't find the corresponding spkg? Then "standard python installs" don't need to be maintained as spkgs, but people still have a transparent way to install them.

(of course, that presumes we have pip as a package---perhaps that's the first external package to be installed...)

[simon-king-jena]

comment:11

Replying to @jasongrout:

(of course, that presumes we have pip as a package---perhaps that's the first external package to be installed...)

That's what someone (actually William, IIRC) has suggested on sage-devel.

[jasongrout]

comment:12

Yep, +1 to that. I've wanted pip to be a standard spkg for a long time.

[rwst]

Work Issues: rebase

[kcrisman]

comment:16

pip is in the meantime standard. See also #8757. Where is sqlalchemy used, anyway (in the Sage library, I mean, not disregarding Simon's point).

[kcrisman]

comment:17

See also https://groups.google.com/forum/#!topic/sage-notebook/LiQEgxT3TWo for removing sqlalchemy altogether.

[kcrisman]

comment:18

See also #4268, which is probably invalid at this point.

[jdemeyer]

comment:19

Shouldn't "sqlalchemy should not be a standard package" and "gdmodule should not be a standard package" be 2 different tickets?

[kcrisman]

comment:20

Probably at this point, yes.

[kcrisman]

comment:21

By the way, sqlalchemy was added in #2205 and this sage-devel thread, where no one ever actually needed it! It was just ... useful. But apparently not enough so to use it for any of our databases.

[kcrisman]

comment:22

See #17591 for gdmodule.

[edd8e884-f507-429a-b577-5d554626c0fe]

comment:23

I am finishing a kind of audit of packages in Sage, and noticed sqlalchemy was useless (and bitrotting for a while). Since there is no interaction with Sage (nor sagenb), i think removing it is a better option than updating it and make it optional. There will be less work to do, and sage -pip install sqlalchemy will let the user have the latest version (or even any other version if needed).

[kcrisman]

comment:24

That's fine, I just figured people who know more about this than I do should be the ones to change it to 'remove' :)

[jdemeyer]

Changed branch from u/ohanar/optionalize to u/jdemeyer/ticket/15593

[jdemeyer]

comment:26

I haven't actually tested it, but this should work...

---

New commits:

2c40f5e

Remove sqlalchemy

[jdemeyer]

Changed author from R. Andrew Ohana to R. Andrew Ohana, Jeroen Demeyer

[jdemeyer]

Changed commit from d701c15 to 2c40f5e

[kcrisman]

comment:27

I would actually not get rid of the line in the db file, since (in principle) someone ambitious could do that someday and it's only in a todo list. We aren't saying we hate sqlalchemy, just that there is no reason to include it in sage-the-distro right now.

[edd8e884-f507-429a-b577-5d554626c0fe]

Changed work issues from rebase to none

[edd8e884-f507-429a-b577-5d554626c0fe]

Description changed:

--- 
+++ 
@@ -1 +1,2 @@
-They are not really used in any core way in the sage distribution (nor or they mathematics/scientific software).
+It is not used in sage nor sagenb and can easily be installed with pip.
+

[jdemeyer]

comment:29

make distclean && make ptestlong passed without problems

[kcrisman]

comment:30

I don't really care about this so much, but just be sure that sagenb does indeed work fine (though I have no reason to expect it not to!).

[videlec]

Reviewer: Vincent Delecroix

[videlec]

comment:31

I just rebuild Sage, testall, reinstall the notebook and ran it...

Vincent

[edd8e884-f507-429a-b577-5d554626c0fe]

Changed branch from u/jdemeyer/ticket/15593 to u/tmonteil/ticket/15593

[edd8e884-f507-429a-b577-5d554626c0fe]

Changed reviewer from Vincent Delecroix to Vincent Delecroix, Karl-Dieter Crisman, Thierry Monteil

[edd8e884-f507-429a-b577-5d554626c0fe]

New commits:

7b582a5

#15593 rewiewer comment 27.

[edd8e884-f507-429a-b577-5d554626c0fe]

Changed commit from 2c40f5e to 7b582a5

[videlec]

comment:34

Thanks Thierry. I somehow skip the comment of Karl-Dieter.

[vbraun]

Changed branch from u/tmonteil/ticket/15593 to 7b582a5