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

### comment:2 follow-up: ↓ 3 Changed 5 years ago by

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: ↓ 4 Changed 5 years ago by

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

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

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

doesn't work?

### comment:6 follow-up: ↓ 8 Changed 5 years ago by

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

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: ↓ 9 Changed 5 years ago by

Replying to vbraun:

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

If you care about `Element`

s, 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: ↓ 10 Changed 5 years ago by

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

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.

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`

...