Standardize MatrixGroup_gap: by adding .cardinality, and deprecating __len__

[nthiery]

Followup on #5308:

• cardinality now returns the size of the group (was order)
• order is a backward compatibility alias for cardinality
• __len__ raises a deprecation error

CC: @sagetrac-sage-combinat @wdjoyner

Component: algebra

Keywords: cardinality, len, order

Author: Nicolas M. Thiéry

Reviewer: David Joyner

Merged: 4.0.2.alpha0

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

[nthiery]

Attachment: matrix_group_gap-cardinality_len-6250-nt.2.patch.gz

[nthiery]

comment:1

matrix_group_gap-cardinality_len-6250-nt.2.patch superceedes the previous one (should have replaced it)

#4326 depends on it

[nthiery]

Description changed:

--- 
+++ 
@@ -1,4 +1,3 @@
-
 Followup on #5308:
 - cardinality now returns the size of the group (was order)
 - order is a backward compatibility alias for cardinality

[wdjoyner]

comment:2

I have not applied or tested this patch but upon reading the code a few lines struck me as odd. Can you please explain how, if F is a finite field, F.order() -> F.cardinality() is correct? Has the order method for finite fields been rewritten? Did I miss the discussion on sage-delev that order should be deprecated and replaced by cardinality?

[nthiery]

comment:3

As far as I remember, what was discussed on the list was about the order of an element of a field/group/...

Here this is about the order of the group itself, which is its cardinality.

[wdjoyner]

comment:4

I don't agree with the suggestion in one of the docstrings that order might be deprecated. But that is just my (American) opinion, which might not be shared by the rest of the world:-) In any case, the patches do not apply cleanly to 4.0.rc0.

[nthiery]

comment:5

Replying to @wdjoyner:

I don't agree with the suggestion in one of the docstrings that order might be deprecated.
But that is just my (American) opinion, which might not be shared by the rest of the world:-)

I am fine with both options. From discussions on sage-devel, it seems that in general aliases are somewhat frowned upon.
We definitely want .cardinality(). But yes, even in Europe, some users would certainly be trying G.order() to get the size
of the group. That's why I raised the issue.

I am happy to remove the comment if you think its better.

In any case, the patches do not apply cleanly to 4.0.rc0.

? I just retried, and it applies smoothly on sage 4.0.1. Did you only apply the second patch? (the first one should be deleted)

[wdjoyner]

comment:6

The second patch applied cleanly to 4.0.rc0 but failed sage -testall. I think it was unrelated but I'll retry on another version of Sage.