Improve @perm macro

[lgoettgens]

Resolves #3010.

• @perm n+1 [....] works now; the docstring always stated it would, but some macro hygiene thing messed up the use of local variables inside the degree expression.
• @perm G [(1,2,3,4)] creates a list of perms with fixed parent G.
• [@perm (1,2), @perm(3,4)] and similar constructions (where julia infers weird scopes of things) now raise an explicit error.
• Complete feature parity for different versions. There are now 9 combinations based on the parent group (detect parent / given group / given degree of symmetric group) and the expression (single permutation, non-empty vector, non-empty tuple). The only ambiguity would be with the empty tuple, so we explicitly restrict vectors and tuple to be non-empty.

The release notes could include a compressed version of this list.

[codecov]

Codecov Report

Attention: Patch coverage is 84.21053% with 6 lines in your changes missing coverage. Please review.

Project coverage is 84.54%. Comparing base (29c57c9) to head (48a6ddf).
Report is 13 commits behind head on master.

Files with missing lines	Patch %	Lines
src/Groups/perm.jl	84.21%	6 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4490      +/-   ##
==========================================
- Coverage   84.60%   84.54%   -0.06%     
==========================================
  Files         672      672              
  Lines       88742    88907     +165     
==========================================
+ Hits        75083    75170      +87     
- Misses      13659    13737      +78     

Files with missing lines	Coverage Δ
src/Groups/perm.jl	96.00% <84.21%> (-2.85%)	⬇️

... and 16 files with indirect coverage changes

[lgoettgens]

@fingolfin this now includes everything we discussed this afternoon

[fingolfin]

allequal requires Julia >= 1.8 and we still run some of these tests in Julia 1.6.

Perhaps we should provide an allequal implementation for older Julia versions?

[lgoettgens]

Isn't this what Compat.jl is for? We already have it in the environment as a dependency of GAP.jl anyway. For the time being, I just import this one function in older julia versions.

(I don't understand why the 1.10 job succeeded, as the allequal dispatch with predicate was only added right before the 1.11 feature freeze)

[benlorenz]

(I don't understand why the 1.10 job succeeded, as the allequal dispatch with predicate was only added right before the 1.11 feature freeze)

1.6 does not have allequal at all and fails, 1.10 does have it and since we load Compat.jl it adds that version to the existing function:

julia> allequal(
allequal(f, xs::Tuple) @ Compat ~/.julia/packages/Compat/gd6AI/src/Compat.jl:790
allequal(f, xs) @ Compat ~/.julia/packages/Compat/gd6AI/src/Compat.jl:789
allequal(r::AbstractRange) @ Base set.jl:537
allequal(c::Union{AbstractDict, AbstractSet}) @ Base set.jl:535
allequal(itr) @ Base set.jl:533

(this is julia 1.10 with Oscar master)

[lgoettgens]

Thanks for reminding me of JuliaLang/Compat.jl#814. This indeed resolves my confusion.

[fingolfin]

Great! Some minor comments. Overall looks good.

[fingolfin]

Suggested change

	evalutating the expressions in the cycle entries multiple times, so it is
	evaluating the expressions in the cycle entries multiple times, so it is

OK for now, but we could avoid that, couldn't we? By storing the result of the evaluation immediately as vectors (of vectors), and then later e.g. passing those to cperm

[lgoettgens]

We can only avoid it for the case of a single permutation, and indeed the previous code did. I removed that for the time being to reduce Tha number of case distinctions.
For the vector and tuple cases, we cannot let the cperm dispatch handle the detection. Consider e.g. @perm [(1, 2), (3,4)]. if we would delegate that to cperm([1, 2]) and cperm([3,4]), then the parents would differ (Sym(2) vs. Sym(4).

I have some ideas on how to avoid the double evaluation, but I need to think a bit more about it and test if it regressed performance for the case of just ints in all entries. Thus, I would like to move that to a follow-up PR