Rename conjugacy_classes_maximal_subgroups and maximal_subgroup_classes or even just *subgroups

[fingolfin]

We discussed it before: conjugacy_classes_maximal_subgroups is hard to find via TAB completion, which we could solve by calling it maximal_subgroup_classes.

Likewise for conjugacy_classes_subgroups -> subgroup_classes.

The old names could be kept but deprecated.

---

If we have this, is there even need for subgroup_reps? It would be equivalent to map(representative, subgroup_classes(g)

Similar for maximal_subgroups.

---

Going one step further (we also discussed this before...), perhaps we should indeed just call these subgroups and maximal_subgroups? Anyone who really wants all (maximal) subgroups has to do a little bit more work then, but otherwise it would steer people into the right direction (in many (most?) applications it is better to work with conjugacy classes). I think this is what Magma does.

IMHO this would also remove the need for subgroup_reps / maximal_subgroup_reps

---

In addition, I think also all other *subgroup_reps functions should be adjusted to provide similar features / variants as subgroups and maximal_subgroups

• hall_subgroup_reps: either add hall_subgroup_classes and hall_subgroups (returning a vector of groups), or just add hall_subgroup and let it return a vector of conjugacy class
• low_index_subgroup_reps: ditto
• complement_class_reps: ditto

[ThomasBreuer]

The following can be done in a non-breaking way:

• hall_subgroup_reps: deprecate, instead provide hall_subgroups, a vector of conjugacy classes of subgroups
• low_index_subgroup_reps: deprecate, instead provide low_index_subgroups, a vector of conjugacy classes of subgroups
• complement_class_reps: deprecate, instead provide complements, a vector of conjugacy classes of subgroups

The following changes would be breaking:

• concerning maximal subgroups:
  deprecate maximal_subgroup_reps and conjugacy_classes_maximal_subgroups
  change the meaning of maximal_subgroups to return a vector of conjugacy classes of the maximal subgroups,
  change the code where maximal_subgroups is used to get a vector of all maximal subgroups

• concerning all subgroups:
  Besides the method for GAPGroups, which returns a vector of all subgroups (without embeddings), there are two methods from Hecke.jl which return (iterators for) subgroups with prescribed properties, together with embeddings.
  If we keep the function name and the functionality for the methods from Hecke.jl, we cannot define subgroups to return always the vector of all conjugacy classes of subgroups.

[thofma]

The iterator version is quite important for our applications. Should we separate it into two methods? subgroups and subgroups_iterator?

[thofma]

What I always find irritating, is that now S = subgroups(G)[1]; order(S) will probably work, but give the answer I was not looking for. I always thought the idea was have the least surprise with the method names, which is why we want is_isomorphic not returning a map anymore.

[ThomasBreuer]

@thofma Thanks for your comments.

The iterator version is quite important for our applications. Should we separate it into two methods? subgroups and subgroups_iterator?

If I understand this idea right then it means that subgroups can be defined to return always the conjugacy classes of subgroups, and subgroups_iterator provides the parameter that restrict the search.

What I always find irritating [...]

One argument is that Magma does the same: Magma's Subgroups returns objects that describe classes of (certain) subgroups.
The point is that when one wants to check something for all subgroups (or all maximal subgroups, etc.) of a given group, it is usually sufficient to check one representative for each conjugacy class, thus asking for all subgroups is in most cases the wrong question.
GAP wanted self-explanatory names, which led to MaximalSubgroups, MaximalSubgroupClassReps, ConjugacyClassesMaximalSubgroups --correct and self-explanatory, but likely a user chooses the "wrong" function.

[thofma]

Magma's Subgroups Function returns a list of records, which makes #Subgroups(G)[1] error and not give an unexpected result.

[ThomasBreuer]

@thofma Good point.
Here is another example where using "class-free" names for functions that return classes yields code that gives wrong impressions when one reads it:

x in maximal_subgroups(G)[1].

This looks as if one asks whether x is an element in some maximal subgroup of G.
However, if maximal_subgroups returns a vector of classes then this means the question whether x is a subgroup of G that lies in the first of these classes.

[thofma]

Maybe there won't be an order(::ConjugacyClass) but maybe length()? Then my example is not really relevant.

On the other hand, I would expect that the "expert" knows that the conjugacy classes are need and thus would probably use maximal_subgroups_classes (or whatever it is called). An "ordinary" user would probably be quite surprised by the output of maximal_subgroups and the subsequent order(...) computations.

[ThomasBreuer]

Yes, currently the conjugacy classes of elements and subgroups in groups do not have order (but length).
My in example is still valid.
(And it is quite possible that sooner or later somebody will argue (for uniformity reasons or so) that it would be a good idea to support also order for classes.)

[fingolfin]

All in all I am fine with the compromise that we have *subgroup_classes functions and matching *subgroups functions; the latter just implemented in terms of the former -- and of course the docstrings should reference the respective counterparts; and the non-classes variants should have a warning/recommendation to consider using the classes variants (and why)

In addition (but this optional and should not block a change if we can't agree oni t), I would let non-classes variants not return a vector but "just" an iterator, i.e.:

subgroups(G::GAPGroups) = Iterators.flatten(subgroup_classes(G))

so that subgroups returns an iterator, not a vector. That way at least some computations are deferred. And one can always use collect on it like everywhere else?

[ThomasBreuer]

I think this is resolved by #3291.