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: |
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
Cc: | Keshav Kini added |
---|
comment:2 Changed 11 years ago by
Priority: | critical → major |
---|
comment:3 follow-up: 5 Changed 11 years ago by
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:5 Changed 11 years ago by
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
Milestone: | sage-5.11 → sage-5.12 |
---|
comment:7 Changed 9 years ago by
Milestone: | sage-6.1 → sage-6.2 |
---|
comment:8 Changed 9 years ago by
Milestone: | sage-6.2 → sage-6.3 |
---|
comment:9 Changed 8 years ago by
Milestone: | sage-6.3 → sage-6.4 |
---|
I guess the priority "major" is enough. After all, we are not talking about errors.