Oscar.jl
Oscar.jl copied to clipboard
`subgroups` command
@wdecker just stumbled over this usability issue.
In short, the subgroups
command is confusing because it does different things for "GAP groups" and abelian groups.
Consider:
help?> subgroups
search: subgroups psubgroups subgroup_reps hall_subgroups_representatives normal_subgroups minimal_subgroups
subgroups(G::GrpGen; order::Int = 0,
index::Int = 0,
normal::Bool = false,
conjugacy_classes::Bool = false)
Returns subgroups of G with embedding.
─────────────────────────────────────────────────────────────────
subgroups(g::GrpAbFinGen;
subtype = :all ,
quotype = :all,
index = -1,
order = -1)
Return an iterator for the subgroups of G of the specific form.
─────────────────────────────────────────────────────────────────
subgroups(G::Group)
Return the vector of all subgroups of G.
While technically the signatures suggest that not every group type supports all, cleaning this requires knowledge of what GrpAbFinGen
and GrpGen
are and are not.
The other issues is that the former two methods return iterators, where each entry is a subgroup together with an embedding, while the latter just returns a vector of subgroups (which technically is also iterable; but the "embedding" bit is missing).
Of course the two first methods are also inconsistent with what they support and do not support, and don't document the meaning of the various kwargs...
One way to improve this would be to at least somewhat align these, i.e.,
- teach
subgroups
for GAPGroups to also at least suppor theindex
andorder
kwargs - maybe also
conjugacy_classes
andnormal
(after figuring out what they really do -- I have guesses but details matter) - return pairs of group, embedding after all (???) -- though I'd prefer if we instead finally implemented the
canonical_map
proposal which allows retrieving these later if needed (but of course I am heavily influenced here because for "my" groups and applications, I almost never need them).
Implementation could just be naive, i.e. "compute all subgroups and discard unwanted ones" -- hey, not ideal, but a start.
In any case, the GAP Groups docstring for subgroups
should also contain a warning that suggests that in many cases it is better to use conjugacy_classes_subgroups
.
A radical alternative would be to change the subgroups
method for GAPGroups to take arbitrary kwargs, and then just do error("not supported for non-abelian groups, use XYZ instead"
).