Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove zodb from sage #10353

Closed
williamstein opened this issue Nov 29, 2010 · 32 comments
Closed

Remove zodb from sage #10353

williamstein opened this issue Nov 29, 2010 · 32 comments
Assignees
Milestone

Comments

@williamstein
Copy link
Contributor

zodb only use the pickle protocol version 1. It is also not used in sage apart from one package that has been migrated away from it in #12205.

There is a zodb mailing list discussion related to the pickle protocol here: http://www.mail-archive.com/[email protected]/msg04628.html

In particular, pickle protocol 1 is hardcoded. But SageObjects/Cython objects must use protocol 2 in many cases:

sage: import cPickle
sage: F = factor(12)
sage: cPickle.dumps(F, protocol=0)
TypeError: can't pickle SageObject objects
sage: cPickle.dumps(F, protocol=1)
TypeError: can't pickle SageObject objects
sage: cPickle.dumps(F, protocol=2)
'\x80\x02csage.structure.factorization\nFactorization\nq\x01)\x81q\x02}q\x03(U\x12_Factorization__crq\x04\x89U\x14_Factorization__unitq\x05csage.rings.integer\nmake_integer\nq\x06U\x011\x85Rq\x07U\x18_Factorization__universeq\x08csage.rings.integer_ring\nIntegerRing\nq\t)Rq\nU\x11_Factorization__xq\x0b]q\x0c(h\x06U\x012\x85Rq\rK\x02\x86q\x0eh\x06U\x013\x85Rq\x0fK\x01\x86q\x10eub.'

See also the related ticket #10352.

The suggested option for this ticket is to remove zodb from sage.


Depends on #12205
Depends on #13717
Depends on #13963

CC: @kini

Component: misc

Author: François Bissey

Reviewer: Jeroen Demeyer

Merged: sage-5.7.beta1

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

@cschwan
Copy link
Mannequin

cschwan mannequin commented Oct 3, 2011

comment:1

Please do not feel offended of my naive question, but what do you think of completely removing ZODB? As far as I understand (by looking at the failing doctests when removing ZODB), only Cremona's database and the Conway polynomials database use it. Ticket #11587 migrates the Cremona's DB to SQLite, so would it not be possible to the same with the Polynomial DB? Are there any optional SPKGs which do depend on ZODB?

@williamstein
Copy link
Contributor Author

comment:2

I added ZODB to Sage (long ago), and I am definitely +1 to removing it from Sage, if we can figure out how. I think it is a technology whose value (for Sage!) has come and gone. One can do better with SQLite these days.

@kiwifb
Copy link
Member

kiwifb commented Oct 3, 2011

comment:3

OK it is really a pain on sage-on-gentoo side and will become worse in a few months (removal from the main tree, zope maintenance is apparently hard that it will be put in its own repo for people who are interested in it).

So I am putting my hand up to remove zodb from sage. Anything else apart from conway polynomial database needs to be cleaned and converted to sqlite?

@kiwifb
Copy link
Member

kiwifb commented Dec 31, 2012

comment:5

It looks like stein-watkins-ecdb also use zodb.

@kiwifb
Copy link
Member

kiwifb commented Jan 3, 2013

comment:6

Hum it looks like sage/databases/stein-watkins-ecdb.py imports sage.databases.db but it is not actually using any of it as far as I can see. So it will be a simple patch to get rid of db.py and compressed_storage.py.

@jpflori
Copy link

jpflori commented Jan 4, 2013

comment:7

If I'm not wrong, sagenb uses some Python packages which are currently included in zodb spkg (I think zope.interface or stg like that, see #10352).

@kiwifb
Copy link
Member

kiwifb commented Jan 4, 2013

comment:8

If I remember the discussion correctly the dependency on zope.interface will be included in a future version of sagenb. My quick check on the current version of sagenb show no import from zope.
It was suggested that it should be included in the sagenb bundle, I personally agree with the idea. In any case that would mean replacing the current zodb spkg with a new one cotaining only zope.interface if we want to keep it in main line.

@kiwifb
Copy link
Member

kiwifb commented Jan 4, 2013

Attachment: trac10353-db-removal.patch.gz

this patch remove reference to zodb from the sage library

@kiwifb
Copy link
Member

kiwifb commented Jan 4, 2013

comment:9

I just attached a patch to remove zodb references from the sage library. I haven't done the patch for sage-root yet.

@kiwifb

This comment has been minimized.

@kiwifb
Copy link
Member

kiwifb commented Jan 5, 2013

Author: François Bissey

@kiwifb
Copy link
Member

kiwifb commented Jan 5, 2013

Dependencies: 12205

@kiwifb kiwifb changed the title ZODB is basically useless in Sage, since it uses pickle protocol 1 Remove zodb from sage Jan 5, 2013
@kini
Copy link
Contributor

kini commented Jan 5, 2013

Changed dependencies from 12205 to #12205

@jdemeyer
Copy link

jdemeyer commented Jan 7, 2013

comment:12

I believe the line in spkg/standard/deps

$(INST)/$(SQLALCHEMY): $(INST)/$(ZODB)

should be removed completely.

@kiwifb
Copy link
Member

kiwifb commented Jan 7, 2013

comment:13

You are right! I probably produced this a bit too close to midnight. I will up a corrected patch shortly.

@kiwifb
Copy link
Member

kiwifb commented Jan 7, 2013

remeve reference to zodb in sage_root

@kiwifb
Copy link
Member

kiwifb commented Jan 7, 2013

comment:14

Attachment: trac10353-sage_root.patch.gz

Patch updated.

@jdemeyer

This comment has been minimized.

@jdemeyer
Copy link

comment:16

This causes build problems for sagenb:

Processing dependencies for Twisted==12.1.0
Searching for zope.interface

Link to http://pypi.python.org/simple/zope.interface/ ***BLOCKED*** by --allow-hosts

Couldn't find index page for 'zope.interface' (maybe misspelled?)
Scanning index of all packages (this may take a while)

Link to http://pypi.python.org/simple/ ***BLOCKED*** by --allow-hosts

No local packages or download links found for zope.interface
error: Could not find suitable distribution for Requirement.parse('zope.interface')
Error installing Twisted-12.1.0.tar.bz2 !

@jdemeyer
Copy link

Work Issues: sagenb

@jpflori
Copy link

jpflori commented Jan 15, 2013

comment:17

I think this was expected and is sagemath/sagenb#126.
So we should open a ticket on Sage's trac and put it as a dependency here I guess.

@jdemeyer
Copy link

Changed dependencies from #12205 to #12205, sagenb-???

@jdemeyer jdemeyer removed this from the sage-5.6 milestone Jan 15, 2013
@jdemeyer
Copy link

Changed dependencies from #12205, sagenb-??? to #12205, #13717

@kini
Copy link
Contributor

kini commented Jan 16, 2013

comment:20

I think this is already fixed, we just need to make a new sagenb tarball. (The only thing sagenb needs to do is package zope.interface, right?)

@kini
Copy link
Contributor

kini commented Jan 16, 2013

comment:21

Ah, looks like this is already fixed in sagenb 0.10.3 (#13717), and the zope.interface problem was noticed in #12719 a couple months ago because IPython has also stopped using zope.

@kini
Copy link
Contributor

kini commented Jan 16, 2013

comment:22

And now that I look more closely at this ticket's dependencies, Jeroen seems to have already noticed that... sorry for the noise :)

@jdemeyer
Copy link

Changed work issues from sagenb to none

@jdemeyer
Copy link

Changed dependencies from #12205, #13717 to #12205, #13717, #13963

@jdemeyer
Copy link

comment:25

I don't feel like reviewing the Sage library patch, but it builds and works fine now.

@kiwifb
Copy link
Member

kiwifb commented Jan 19, 2013

comment:26

It's just removing files and one line that is useless. Christopher, could you give a review to this?

@jdemeyer
Copy link

Reviewer: Jeroen Demeyer

@jdemeyer jdemeyer added this to the sage-5.7 milestone Jan 25, 2013
@jdemeyer jdemeyer removed the pending label Jan 25, 2013
@jdemeyer
Copy link

Merged: sage-5.7.beta1

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants