cleaning CategoryObject/Parent

[videlec]

Abstract

This ticket stands to clean CategoryObject and Parent with respect to:

• hacks and too specialized methods (e.g. base or _ngens_)
• undocumented and old (e.g. sage.structure.generators)
• duplicated stuff (e.g. _an_element_ and an_element)

Subtasks

Base and generators

We should move out of CategoryObject the attributes and methods related to generators and variable names.

Attributes:

• _names

Methods:

• gens_dict
• gens_dict_recursive
• objgens
• objgen
• _first_ngens
• _defining_names
• _assign_names
• __temporarily_change_names
• variable_names
• variable_name
• latex_variable_names
• latex_name
• inject_variables
• base_ring

It is not clear where all this should be moved... Moreover "generators" is ambiguous since "monoid generators", "group generators", "algebra generators", ... are different things.

Keep

These were originally proposed to be removed too, but are sufficiently useful:

Attributes:

• _base
  Methods:
• base
  Functions:
• normalize_names
• certify_names

Tickets

Some related tickets are

• gradual transition for to remove inheritance from ParentWithGens to Ring (New class for gradually transitioning parents with generators to the new coercion model #13683? still useful?)

• normalization: variable vs variable_name vs variables #24430: normalization for _names, variable_name and variable_names in the context of polynomials and series

Transition to FiniteEnumeratedSet

• set the category to FiniteEnumeratedSets where appropriate (Categories for finite (enumerated) sets #12957)

• Move __len__ and __getitem__ from Parent to FiniteEnumeratedSets (Move __len__ and __getitem__ from Parent to FiniteEnumeratedSets #12955)

• deprecate CombinatorialClass in favor of the EnumeratedSets category (Meta-ticket: Deprecate CombinatorialClass in favor of the EnumeratedSet's categories #12913, make Combinations a new style Parent #19986)

Duplicated names for same function

• cleanup an_element (Cleanup an_element #18291)

• cardinality vs order (cardinality vs order #18410)

Others

• move Set_generic and Set_Python_type_class out of sage.structure.parent. Moreover we should use UniqueRepresentation instead of the custom class factory Set_PythonType.

• Various fixes in category initialization (fix MIPVariable inheritance #21353)

• cleanup variable factories (cleanup variable factories #18390)

• cleanup cartesian products (task ticket Meta-ticket: Cleanup cartesian products #15425)

Closed tickets

• refactor getattr_from_other_class (Refactor getattr_from_other_class() for lookup of methods in categories #20686)

• Simplify _populate_generators_ (Simplify _populate_generators_ #21381)

• Remove sage.structure.generators (Remove sage.structure.generators #21382)

• Remove ParentWith*AbelianGens (Remove ParentWith*AbelianGens and Module_old #21383)

• Remove _populate_generators_ (Remove support for Parent.__init__(gens=...) #21385)

• Always enable debug.bad_parent_warnings (Always enable debug.bad_parent_warnings #24109)

CC: @tscrim

Component: categories

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

[jdemeyer]

comment:1

Are you sure this would affect Element much? All the things you write are about CategoryObject or Parent.

[jdemeyer]

Replying to @videlec:

We should move out of CategoryObject the attributes and methods relative to "base"

After thinking about this now and then, I think that it does make sense to keep _base inside CategoryObject. There are a lot of objects which naturally have a base object. For efficiency and simplicity, I suggest to keep CategoryObject._base.

[jdemeyer]

comment:21

What is wrong with the functions normalize_names() and certify_names? They have to be put somewhere, why not in structure/category_object.pyx?

[videlec]

comment:22

Replying to @jdemeyer:

Replying to @videlec:

We should move out of CategoryObject the attributes and methods relative to "base"

After thinking about this now and then, I think that it does make sense to keep _base inside CategoryObject. There are a lot of objects which naturally have a base object. For efficiency and simplicity, I suggest to keep CategoryObject._base.

Some does, some does not and there is no specification of what a base is. I would be happy to hear a definition.

[videlec]

comment:23

Replying to @jdemeyer:

What is wrong with the functions normalize_names() and certify_names? They have to be put somewhere, why not in structure/category_object.pyx?

Right (I made the listing rather quickly)

[jdemeyer]

comment:24

Replying to @videlec:

Some does, some does not and there is no specification of what a base is. I would be happy to hear a definition.

Well, we cannot define "base" in full generality. Some objects do not have a natural base and then this attribute should simply be None.

But in certain important categories, we can define "base". For example, for modules, we define "base" to mean the parent of the scalars used for scalar multiplication.

[videlec]

comment:25

Replying to @jdemeyer:

Replying to @videlec:

Some does, some does not and there is no specification of what a base is. I would be happy to hear a definition.

Well, we cannot define "base" in full generality. Some objects do not have a natural base and then this attribute should simply be None.

But in certain important categories, we can define "base". For example, for modules, we define "base" to mean the parent of the scalars used for scalar multiplication.

This is precisely the reason why I dislike it. It is a fake global concept that is mostly meant for "the underlying ring of a module". And because modules are important all parents have an attribute _base as well as methods base and base_ring. Think for a minute about the list of parents without base: permutations, graphs, groups, rings, schemes, simplicial complexes, ...

Moreover, I (maybe wrongly) thought that this "base ring of a module" is the kind of information that belongs to the category

sage: Modules(ZZ)
Category of modules over Integer Ring
sage: Modules(ZZ).base()
Integer Ring

[jdemeyer]

comment:39

Replying to @videlec:

Moving base() and base_ring() to the category would not be possible if the attribute _base is private. How do you plan to declare it?

You could make _base accessible to Python code (cdef public _base instead of cdef _base).

[jdemeyer]

comment:40

Replying to @tscrim:

I was thinking of a hierarchy like this:

+-Parent
|
+---ParentWithBase  (maybe useless right now)
|
+-+-ParentWithGens
  |
  +-+-ParentWithBaseAndGens
    |
    +-Module (etc).

I don't think that it makes much sense to have unrelated classes ParentWithBase and ParentWithBaseAndGens. Since base seems more important than gens, I would actually suggest

CategoryObject
|
+--Parent
   |
   +--ParentWithBase
      |
      +--Module
      |
      +--Ring
      |
     ...

I said new-style because there still is the old-style ParentWithGens floating around.

Thanks for the reminder. Getting rid of those old-style parents should probably be higher priority than this ticket.

[tscrim]

comment:41

Replying to @jdemeyer:

Replying to @tscrim:

I was thinking of a hierarchy like this:

+-Parent
|
+---ParentWithBase  (maybe useless right now)
|
+-+-ParentWithGens
  |
  +-+-ParentWithBaseAndGens
    |
    +-Module (etc).

I don't think that it makes much sense to have unrelated classes ParentWithBase and ParentWithBaseAndGens.

Ideally it should be a diamond IMO, but Cython does not (yet?) support multiple inheritance (or something like interfaces in Java-speak).

Since base seems more important than gens, I would actually suggest

CategoryObject
|
+--Parent
   |
   +--ParentWithBase
      |
      +--Module
      |
      +--Ring
      |
     ...

We have objects with base but without gens (e.g., polytopes), objects without base but with gens (e.g., permutation groups), as well as with(out) both. So I was trying to address that with my proposed hierarchy. At least I would like ParentWithGens in between Parent and ParentWithBase as there are very few things in Sage that have gens but no concept of base. (Which, as it turns out, is not quite the old parent hierarchy ParentWithGens -> ParentWithBase -> Parent_old.)

Although I'm a little worried with the group class hierarchy becoming a bit more of a mess because of the Cython parents and not having the multiple inheritance. Might be a necessary complication in order to improve the interface (which IMO is net benefit). Spreading out to a more refined parent hierarchy is going to be a fairly invasive change, so I guess it will just have to be done.

I said new-style because there still is the old-style ParentWithGens floating around.

Thanks for the reminder. Getting rid of those old-style parents should probably be higher priority than this ticket.

Yes, definitely. Although most of them are really fundamental parents such as Ring, number fields, etc. I guess we can take care of the leaf classes a little more easily, such as ComplexIntervalField. Although it is somewhat counter to the objective of having a more refined parent hierarchy as it usually means making the class inherit from Parent...

[jdemeyer]

comment:42

Replying to @tscrim:

Cython does not (yet?) support multiple inheritance

First of all, the next release of Cython is going to support multiple inheritance in a limited way. The list of bases can be CyClass, PyClass1, ..., PyClassN, which is the most general thing which is allowed by Python and which doesn't hurt Cython's performance (because all Python classes come after all Cython classes in the MRO).

Second, the fact that diamonds are not allowed is because of Python's design. In plain Python, you cannot do class X(list, str) either. So don't blame Cython for something that they cannot fix.

Anyway, the conclusion is that ParentWithGens could be a Python class and then the diamond would work in Cython 0.28.

[jdemeyer]

comment:43

Replying to @tscrim:

Yes, definitely. Although most of them are really fundamental parents such as Ring

The large majority of rings use the new coercion model, so it's not as bad as you think.

[tscrim]

comment:44

Replying to @jdemeyer:

Replying to @tscrim:

Cython does not (yet?) support multiple inheritance

First of all, the next release of Cython is going to support multiple inheritance in a limited way. The list of bases can be CyClass, PyClass1, ..., PyClassN, which is the most general thing which is allowed by Python and which doesn't hurt Cython's performance (because all Python classes come after all Cython classes in the MRO).

That's good to hear and should be useful here.

Second, the fact that diamonds are not allowed is because of Python's design. In plain Python, you cannot do class X(list, str) either. So don't blame Cython for something that they cannot fix.

Sorry, I guess I was thinking a little too loosely with not distinguishing between classes and (builtin) types.

Anyway, the conclusion is that ParentWithGens could be a Python class and then the diamond would work in Cython 0.28.

I think this would be the most sensible solution since most of our Cython parent classes would inherit from the ParentWithGensAndBase (e.g., Ring, where we really treat everything as some R-algebra), but would keep the flexibility for other parents in a reasonable way.

[tscrim]

comment:45

Replying to @jdemeyer:

Replying to @tscrim:

Yes, definitely. Although most of them are really fundamental parents such as Ring

The large majority of rings use the new coercion model, so it's not as bad as you think.

That's good to hear. Now if only I could find some time to work on this directly...

[mkoeppe]

comment:48

Setting new milestone based on a cursory review of ticket status, priority, and last modification date.