Dear c-level python specialists,

Nested classes in python are really handy, and work pretty well,
except for a bug in pickling:

Let's define a nested class:

        sage: class Bla:                                                      
        ....:     class Ble:                                                  
        ....:         pass                                                    
        ....:

Let's fake Bla is defined in a module, not in the interpreter:

        sage: import __main__  
        sage: __main__.Bla = Bla

Dumping an element of Bla works just fine:
        sage: x = Bla()
        sage: dumps(x)
        "x\x9ck`\xd2H\x8e\x8f\xcfM\xcc\xcc\x8b\x8f\xe7r\xcaI\xe4*d\xcc/d\xaa-dN\xd2 \x03\x00v\xe3\x08'"

But dumping an element of Bla.Ble breaks:

        sage: x = Bla.Ble()
        sage: dumps(x)
        ---------------------------------------------------------------------------
        PicklingError                             Traceback (most recent call last)

        /home/nthiery/.sage/temp/zephyr/23765/_home_nthiery__sage_init_sage_0.py in <module>()

        /opt/sage/local/lib/python2.5/site-packages/sage/structure/sage_object.so in sage.structure.sage_object.dumps (sage/structure/sage_object.c:5539)()

        PicklingError: Can't pickle __main__.Ble: attribute lookup __main__.Ble failed
        > /opt/sage-3.4.rc0/devel/sage-combinat/sage/sage_object.pyx(571)sage.structu re.sage_object.dumps (sage/structure/sage_object.c:5539)()
        ---------------------------------------------------------------------------

The reason behind this is twofold:

First, the name of the class is improperly set:

        sage: Bla.Ble
        <class __main__.Ble at 0xb1e7c98c>
        sage: Bla.Ble.__name__
        'Ble'

(which btw probably breaks other tools around)

Well, let's work around this:

        sage: Bla.Ble.__name__ = "Bla.Ble"

But then, a second problem: the code of cPickle (around line 1977)
tries to lookup the attribute "Bla.Ble" in __main__, instead of
successively looking up "Bla" in __main__, and then "Ble" in the result:

        sage: dumps(Bla.Ble)
        ---------------------------------------------------------------------------
        PicklingError                             Traceback (most recent call last)
        /home/nthiery/.sage/temp/zephyr/23765/_home_nthiery__sage_init_sage_0.py in <module>()
        /opt/sage/local/lib/python2.5/site-packages/sage/structure/sage_object.so in sage.structure.sage_object.dumps (sage/structure/sage_object.c:5539)()
        PicklingError: Can't pickle __main__.Bla.Ble: attribute lookup __main__.Bla.Ble failed
        > /opt/sage-3.4.rc0/devel/sage-combinat/sage/sage_object.pyx(571)sage.structu re.sage_object.dumps (sage/structure/sage_object.c:5539)()

It would take me a couple hours to learn the syntax to write c-code
for python modules. Could any of the experts out there write me a
2-line patch upon cPickle:1977 to do the right thing (i.e. split
__name__, and make the attribute lookup for each bit)? Just a draft
will be fine; I am fine with reading and simple debuging.

Thanks in advance!

Best regards,
                                Nicolas

Here is the broken code:

        mod = PyImport_ImportModule(module_str);
        if (mod == NULL) {
                cPickle_ErrFormat(PicklingError,
                                  "Can't pickle %s: import of module %s "
                                  "failed",
                                  "OS", args, module);
                goto finally;
        }
        ############### HERE ####################
        klass = PyObject_GetAttrString(mod, name_str);
        if (klass == NULL) {
                cPickle_ErrFormat(PicklingError,
                                  "Can't pickle %s: attribute lookup %s.%s "
                                  "failed",
                                  "OSS", args, module, global_name);
                goto finally;
        }

--
Nicolas M. Thiéry "Isil" <nthi...@users.sf.net>
http://Nicolas.Thiery.name/

On Fri, Mar 13, 2009 at 1:03 AM, Nicolas M. Thiery

May I suggest a different workaround?  (It's gross, but maybe less
gross than changing cPickle.)

sage: __main__.__dict__['Bla.Ble'] = Bla.Ble
sage: loads(dumps(x))
<__main__.Bla.Ble instance at 0x1a2e7eac>

Carl

Well, that's a bug in python that should eventually be fixed in
python. But yeah, your workaround could be a good way out.

Thanks!

Now, you don't have by any chance a magic spell, so that whenever such
a nested class Foo.Bar is created, 'Foo.Bar' gets inserted in its
module as above? (I guess I can hack around for my application).

Cheers,
                                Nicolas
--
Nicolas M. Thiéry "Isil" <nthi...@users.sf.net>
http://Nicolas.Thiery.name/

On Fri, Mar 13, 2009 at 12:55 PM, Nicolas M. Thiery

This is the purpose of metaclasses.  The best introduction / primer on
metaclasses that I've read is at
http://cleverdevil.org/computing/78/

--Mike

On Fri, Mar 13, 2009 at 12:55 PM, Nicolas M. Thiery

I certainly wouldn't call it a bug -- at the most, a missing feature.
I'm not even convinced that a patched version would be an improvement.
 (For one thing, you would need to also patch the unpickle side, which
means that you can't write a pickle using your patch and read it in
older versions of Python.  We don't care much about that in Sage, but
I think it's important to the Python people.)

Carl

        Hi Mike!

Thanks, I'll have a look.

I am already playing with meta classes for UniqueRepresentation and
dynamic classes, and the two of them readily do not collaborate
without special attention. So as of now, I find the concept extremely
fun, but I try to keep its potential uses to a strict minimum :-)

Cheers,
                                Nicolas
--
Nicolas M. Thiéry "Isil" <nthi...@users.sf.net>
http://Nicolas.Thiery.name/

        Dear Carl, dear all,

This is a call for comments and reviews for the new ticket #5986 whose
description is:

--------------------------------------------------------------------------- ---
With the python code below::

class A:
    class B:
        pass

Python 2.6 erroneously set B.__name__ to "B" instead of "A.B".

Furthermore, upon pickling (here in save_global) *and* unpickling (in
load_global) a class with name "A.B" in a module mod, the standard
cPickle module searches for "A.B" in mod.dict instead of looking up
"A" and then "B" in the result.

This patch works around this by a patch to cPickle.c which fixes the
name for B to its appropriate value A.B, and inserts 'A.B' = A.B in
mod.dict (hacky, but seems to work) the first time A.B is pickled, and
fixes load_global to implement a proper lookup upon unpickling.

It also ensures that sage/interfaces/sage0.py uses loads/dumps from
sage_object rather than calling directly cPickle.loads/dumps
(+1 by cwitty on this change)

Python source experts are more than welcome to rework/rewrite this
patch!

Depends on #5483 (explain-pickle) and #5985
--------------------------------------------------------------------------- ---

Thanks in advance!

Best,
                                Nicolas

...

Well, you can't pickle/unpickle it in the first place, so this can't
hurt more than currently.

--
Nicolas M. Thiéry "Isil" <nthi...@users.sf.net>
http://Nicolas.Thiery.name/

On Tue, May 5, 2009 at 12:30 AM, Nicolas M. Thiery

Have you posted this to the python-dev mailing list?  This patch will
definitely not go into Sage unless it will definitely go into Python
at some point in the future. What if you are simply wrong in your
assertion that Python "erroneously set B.__name__ to "B" instead of
"A.B"?"   Then you'll just be making it so Sage can never work with
any unpatched version of Python.   Note that (1) in the entire history
of the Sage project, we have never once patched the Python
interpreter, except to fix small bus we found whose fixes were
upstreamed (the only thing I can think of along those lines is a
readline bug on Itanium), and (2) if we patch Python, then Sage for
Debian/Ubuntu will never be upgraded to the version of Sage with this
patch until the patch is upstreamed into Python.

--
William Stein
Associate Professor of Mathematics
University of Washington
http://wstein.org

Not yet but this of course definitely is the plan.

(sorry, I already have trouble keeping up with my current average
patch submission of one per day).

What a nice way to start a polite negotiation between gentlemens.

(european vs american culture strikes back :-) )

(1) Yes, maybe my assumption is wrong, though I strongly doubt it. In
any cases, what is for sure is that with the current version of
Python, nested classes cannot be pickled. And this is undoubtly a bug
(or missing feature if you prefer). The single purpose of my patch is
to fix this. Any other fix or workaround will do as well, and is more
than welcome.

(2) My patch does not touch the Python interpreter, but a copy of
cPickle within the Sage source tree. This does not prevent upgrading
python, as long as we keep our copy of cPickle reasonably updated. Of
course this is only acceptable as a temporary workaround.

(3) -1 on holding back further a large scale merge for a minor
technical detail that will eventually be resolved one way or the
other.

(4) My general feeling is that nested classes in Python are on the
edge of full maturity, with just a couple glitches here and
there. Typically, Florent mentionned to me that the devel version of
Sphynx now handled them correctly. This is why I feel confident using
them. And there is a clear advantage to using them (which I'll argue
about in my demo in Seattle).

Best,
                                Nicolas
--
Nicolas M. Thiéry "Isil" <nthi...@users.sf.net>
http://Nicolas.Thiery.name/

On Tue, May 5, 2009 at 9:34 AM, Nicolas M. Thiery

Clever.  That's actually a very good idea, which mostly negates my
concerns.  Nonetheless, I would really like to hear from python-dev
about what *they* thinking about pickling and nested classes.  What if
there are some good reasons for the current design that  we're just
not thinking of?

:-)

Credits shared with Carl and Mike!

Definitely!

Best,
                                Nicolas
--
Nicolas M. Thiéry "Isil" <nthi...@users.sf.net>
http://Nicolas.Thiery.name/