Opened 11 years ago

Last modified 8 years ago

#12071 new PLEASE CHANGE

Call `__init__` of base classes

Reported by: Simon King Owned by: tbd
Priority: major Milestone: sage-6.4
Component: performance Keywords: base class __init__
Cc: Keshav Kini Merged in:
Authors: Reviewers:
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: #11900 Stopgaps:

Status badges

Description

Base classes are useful, and they are used. They would be even more useful if one would call their __init__ - but that's often not done in Sage.

For example, even though the class of continued fraction field inherits from the base class sage.rings.ring.Field, it used to only call ParentWithGens.__init__. That is very annoying, since by #9138 Field.__init__ would automatically initialise the correct category.

I fixed a couple of cases at #11900, so, I make this a dependency. Hoewever, according to search_src, there are still many cases left that needs to be looked at:

tensor/differential_forms.py:110:        ParentWithGens.__init__(self, SR, \
groups/group.pyx:55:        sage.structure.parent_gens.ParentWithGens.__init__(self,
rings/real_mpfi.pyx:451:        ParentWithGens.__init__(self, self, tuple([]), False, category = Fields())
rings/multi_power_series_ring.py:305:        ParentWithGens.__init__(self, base_ring, name_list)
rings/complex_field.py:185:        ParentWithGens.__init__(self, self._real_field(), ('I',), False, category = Fields())
rings/integer_ring.pyx:236:        ParentWithGens.__init__(self, self, ('x',), normalize=False, category = EuclideanDomains())
rings/complex_mpc.pyx:297:        ParentWithGens.__init__(self, self._real_field(), ('I',), False)
rings/complex_double.pyx:158:        ParentWithGens.__init__(self, self, ('I',), normalize=False, category = Fields())
rings/complex_interval_field.py:144:        ParentWithGens.__init__(self, self._real_field(), ('I',), False, category = Fields())
rings/infinity.py:472:        ParentWithGens.__init__(self, self, names=('oo',), normalize=False)
rings/infinity.py:783:        ParentWithGens.__init__(self, self, names=('oo',), normalize=False)
rings/real_mpfr.pyx:308:        ParentWithGens.__init__(self, self, tuple([]), False, category = Fields())
rings/rational_field.py:213:        ParentWithGens.__init__(self, self, category = QuotientFields())
rings/number_field/number_field.py:1005:        ParentWithGens.__init__(self, QQ, name, category=category)
coding/linear_code.py:706:        ParentWithGens.__init__(self, base_ring)

Change History (9)

comment:1 Changed 11 years ago by Keshav Kini

Cc: Keshav Kini added

comment:2 Changed 11 years ago by Simon King

Priority: criticalmajor

I guess the priority "major" is enough. After all, we are not talking about errors.

comment:3 Changed 11 years ago by Volker Braun

It was my understanding that this is done intentionally since these parents don't use the new coercion framework, so you get less errors if you circumvent some of the constructors. So really, you are saying rewrite everything for the newer framework :-)

comment:4 Changed 11 years ago by Keshav Kini

Shouldn't that be done anyway?

comment:5 in reply to:  3 Changed 11 years ago by Simon King

Replying to vbraun:

It was my understanding that this is done intentionally since these parents don't use the new coercion framework, so you get less errors if you circumvent some of the constructors. So really, you are saying rewrite everything for the newer framework :-)

I don't think so. It rather seems to me that these are very old bits of code that predate the new coercion framework. Moreover, in the few cases that I fixed at #11900, it was really just "replace ParentWithGens.__init__ with the proper thing to do", and then it worked, modulo some trivial doctest fixes.

comment:6 Changed 9 years ago by Jeroen Demeyer

Milestone: sage-5.11sage-5.12

comment:7 Changed 9 years ago by For batch modifications

Milestone: sage-6.1sage-6.2

comment:8 Changed 9 years ago by For batch modifications

Milestone: sage-6.2sage-6.3

comment:9 Changed 8 years ago by For batch modifications

Milestone: sage-6.3sage-6.4
Note: See TracTickets for help on using tickets.