Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Call __init__ of base classes #12071

Open
simon-king-jena opened this issue Nov 22, 2011 · 4 comments
Open

Call __init__ of base classes #12071

simon-king-jena opened this issue Nov 22, 2011 · 4 comments

Comments

@simon-king-jena
Copy link
Member

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)

Depends on #11900

CC: @kini

Component: performance

Keywords: base class init

Issue created by migration from https://trac.sagemath.org/ticket/12071

@simon-king-jena
Copy link
Member Author

comment:2

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

@vbraun
Copy link
Member

vbraun commented Nov 22, 2011

comment:3

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 :-)

@kini
Copy link
Contributor

kini commented Nov 22, 2011

comment:4

Shouldn't that be done anyway?

@simon-king-jena
Copy link
Member Author

comment:5

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.

@jdemeyer jdemeyer modified the milestones: sage-5.11, sage-5.12 Aug 13, 2013
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.1, sage-6.2 Jan 30, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.2, sage-6.3 May 6, 2014
@sagetrac-vbraun-spam sagetrac-vbraun-spam mannequin modified the milestones: sage-6.3, sage-6.4 Aug 10, 2014
@mkoeppe mkoeppe removed this from the sage-6.4 milestone Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants