Opened 5 years ago

Last modified 5 years ago

#18026 new defect

Parent breakage because not every SageObject is idempotent

Reported by: vbraun Owned by:
Priority: major Milestone: sage-6.6
Component: pickling Keywords:
Cc: jdemeyer, nthiery, vdelecroix Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Description

Every SageObject has a parent by #10962, possibly its own type. But that doesn't mean that P(P(x)) returns P(x), or even succeeds:

sage: Sequence([plot(sin)])
---------------------------------------------------------------------------
TypeError                                 Traceback (most recent call last)
<ipython-input-4-b6d3f68cd8b4> in <module>()
----> 1 Sequence([plot(sin)])

/home/vbraun/Code/sage.git/local/lib/python2.7/site-packages/sage/structure/sequence.pyc in Sequence(x, universe, check, immutable, cr, cr_str, use_sage_types)
    293         return PolynomialSequence(x, universe, immutable=immutable, cr=cr, cr_str=cr_str)
    294     else:
--> 295         return Sequence_generic(x, universe, check, immutable, cr, cr_str, use_sage_types)
    296 
    297 

/home/vbraun/Code/sage.git/local/lib/python2.7/site-packages/sage/structure/sequence.pyc in __init__(self, x, universe, check, immutable, cr, cr_str, use_sage_types)
    498         self.__universe = universe
    499         if check:
--> 500             x = [universe(t) for t in x]
    501         list.__init__(self, x)
    502         self._is_immutable = immutable

TypeError: __init__() takes exactly 1 argument (2 given)

Change History (10)

comment:1 Changed 5 years ago by vdelecroix

And what is the exact issue? Should we force P(P(x)) to work as soon as P(x) does? Could be part of the TestSuite...

comment:2 follow-up: Changed 5 years ago by vbraun

I'm leaning towards not making parent a method of SageObject. Move it to Element. Not everything fits into the parent/element scheme. Parents probably don't need a parent class either:

sage: ZZ.parent()
<type 'sage.rings.integer_ring.IntegerRing_class'>

comment:3 in reply to: ↑ 2 ; follow-up: Changed 5 years ago by vdelecroix

Replying to vbraun:

I'm leaning towards not making parent a method of SageObject. Move it to Element. Not everything fits into the parent/element scheme. Parents probably don't need a parent class either:

sage: ZZ.parent()
<type 'sage.rings.integer_ring.IntegerRing_class'>

The main reason for the introduction of the .parent() method was because of MaximaWrapper which does not inherit from Element. But whatever we do with this method, it is what the function parent is doing

sage: parent(4r)
<type 'int'>
sage: parent(int)
<type 'type'>

And I would not call it a "parent class" since it is the class of the object itself that is considered as a parent. Which makes sense.

By the way, the example you wrote in the description of the ticket used to work in previous versions of Sage?

comment:4 in reply to: ↑ 3 Changed 5 years ago by vbraun

Replying to vdelecroix:

And I would not call it a "parent class" since it is the class of the object itself that is considered as a parent. Which makes sense.

No. Parent calls are idempontent, type calls are not.

comment:5 Changed 5 years ago by jdemeyer

Honestly, I also don't understand the issue. So what if P(P(x)) doesn't work?

comment:6 follow-up: Changed 5 years ago by vbraun

IMHO that is one of the implicit promises of the parent/element pattern: Cast to the current parent gives you the same element e.parent()(e) == e. E.g. the definition of e in e.parent() is along those lines. If you violate that then you'll run into places where that assumption was implicitly made, for example Sequence().

comment:7 Changed 5 years ago by tscrim

However not every SageObject is a parent. For sequences, we have #15852 (which is on my todo list to finish reviewing!), but do we have other SageObject's that should be elements of some parent?

comment:8 in reply to: ↑ 6 ; follow-up: Changed 5 years ago by jdemeyer

Replying to vbraun:

IMHO that is one of the implicit promises of the parent/element pattern

If you care about Elements, then isinstance(foo, Element) is your friend...

You are making the mistake that everything which has a parent() method is an Element of some Parent.

comment:9 in reply to: ↑ 8 ; follow-up: Changed 5 years ago by vbraun

Replying to jdemeyer:

You are making the mistake that everything which has a parent() method is an Element of some Parent.

If it walks like a duck and talks like a duck then it must be a duck.

comment:10 in reply to: ↑ 9 Changed 5 years ago by tscrim

Replying to vbraun:

Replying to jdemeyer:

You are making the mistake that everything which has a parent() method is an Element of some Parent.

If it walks like a duck and talks like a duck then it must be a duck.

The issue to me is that SageObject walks and talks pretty close to a duck, but it is just a hunter with a toy duck trying to lure you in. I agree with Volker in comment:2 that we should strip parent out of SageObject. Also the TestSuite checks currently for idempotent-ness (but IDK if that needs to be a subclass of Parent).

While we are at it, could we also do some of the others like base_ring and n?

Note: See TracTickets for help on using tickets.