Opened 9 years ago

Closed 7 years ago

#10353 closed defect (fixed)

Remove zodb from sage

Reported by: was Owned by: jason
Priority: major Milestone: sage-5.7
Component: misc Keywords:
Cc: kini Merged in: sage-5.7.beta1
Authors: François Bissey Reviewers: Jeroen Demeyer
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #12205, #13717, #13963 Stopgaps:

Description (last modified by jdemeyer)

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/zodb-dev@zope.org/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.


Attachments (2)

trac10353-db-removal.patch (17.8 KB) - added by fbissey 7 years ago.
this patch remove reference to zodb from the sage library
trac10353-sage_root.patch (4.2 KB) - added by fbissey 7 years ago.
remeve reference to zodb in sage_root

Download all attachments as: .zip

Change History (30)

comment:1 Changed 8 years ago by cschwan

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?

comment:2 Changed 8 years ago by was

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.

comment:3 Changed 8 years ago by fbissey

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?

comment:4 Changed 8 years ago by kini

  • Cc kini added

comment:5 Changed 7 years ago by fbissey

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

comment:6 Changed 7 years ago by fbissey

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.

comment:7 Changed 7 years ago by jpflori

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

comment:8 Changed 7 years ago by fbissey

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.

Changed 7 years ago by fbissey

this patch remove reference to zodb from the sage library

comment:9 Changed 7 years ago by fbissey

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

comment:10 Changed 7 years ago by fbissey

  • Authors set to François Bissey
  • Dependencies set to 12205
  • Description modified (diff)
  • Status changed from new to needs_review
  • Summary changed from ZODB is basically useless in Sage, since it uses pickle protocol 1 to Remove zodb from sage

comment:11 Changed 7 years ago by kini

  • Dependencies changed from 12205 to #12205

comment:12 Changed 7 years ago by jdemeyer

I believe the line in spkg/standard/deps

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

should be removed completely.

comment:13 Changed 7 years ago by fbissey

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

Changed 7 years ago by fbissey

remeve reference to zodb in sage_root

comment:14 Changed 7 years ago by fbissey

Patch updated.

comment:15 Changed 7 years ago by jdemeyer

  • Description modified (diff)

comment:16 Changed 7 years ago by jdemeyer

  • Status changed from needs_review to needs_work
  • Work issues set to sagenb

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 !

comment:17 Changed 7 years ago by jpflori

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

comment:18 Changed 7 years ago by jdemeyer

  • Dependencies changed from #12205 to #12205, sagenb-???
  • Milestone changed from sage-5.6 to sage-pending
  • Status changed from needs_work to needs_review

comment:19 Changed 7 years ago by jdemeyer

  • Dependencies changed from #12205, sagenb-??? to #12205, #13717

comment:20 Changed 7 years ago by kini

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?)

comment:21 Changed 7 years ago by kini

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.

comment:22 Changed 7 years ago by kini

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

comment:23 Changed 7 years ago by jdemeyer

  • Work issues sagenb deleted

comment:24 Changed 7 years ago by jdemeyer

  • Dependencies changed from #12205, #13717 to #12205, #13717, #13963

comment:25 Changed 7 years ago by jdemeyer

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

comment:26 Changed 7 years ago by fbissey

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

comment:27 Changed 7 years ago by jdemeyer

  • Milestone changed from sage-pending to sage-5.7
  • Reviewers set to Jeroen Demeyer
  • Status changed from needs_review to positive_review

comment:28 Changed 7 years ago by jdemeyer

  • Merged in set to sage-5.7.beta1
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.