FiniteEnumeratedSets cardinality override

[tscrim]

Right now the category FiniteEnumeratedSets overrides a parent class's cardinality() when creating a list. Below is a minimal example of this behavior:

from sage.structure.parent import Parent
from sage.categories.finite_enumerated_sets import FiniteEnumeratedSets

class TestParent(Parent):
    def __init__(self):
        Parent.__init__(self, category=FiniteEnumeratedSets())

    def __iter__(self):
        yield 1
        return

    def cardinality(self, bad_arg):
        """
        EXAMPLES::

            sage: P = sage.combinat.category_doctest_fail.TestParent()
            sage: P.cardinality(-1)
            1
            sage: v = P.list(); v
            [1]
            sage: len(v)
            1
            sage: P.cardinality()
            1
            sage: P.cardinality(-1) # This test breaks
            1
        """
        return 1 # we don't want to change the semantics of cardinality()

This seems to be caused by not checking if the parent class has a cardinality() function implemented, and just overriding it with _cardinality_from_list() (which takes no [optional/keyword] parameters).

CC: @nthiery @AndrewAtLarge @sagetrac-sage-combinat

Component: categories

Keywords: finite sets, cardinality, days45

Author: Travis Scrimshaw

Reviewer: Andrew Mathas

Merged: sage-5.8.beta1

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

[tscrim]

Description changed:

--- 
+++ 
@@ -1,6 +1,6 @@
 Right now the category `FiniteEnumeratedSets` overrides a parent class's `cardinality()` when creating a list. Below is a minimal example of this behavior:
 
-```
+```python
 from sage.structure.parent import Parent
 from sage.categories.finite_enumerated_sets import FiniteEnumeratedSets
 
@@ -12,13 +12,13 @@
         yield 1
         return
 
-    def cardinality(self, n):
+    def cardinality(self, bad_arg):
         """
         EXAMPLES::
 
             sage: P = sage.combinat.category_doctest_fail.TestParent()
             sage: P.cardinality(-1)
-            -1
+            1
             sage: v = P.list(); v
             [1]
             sage: len(v)
@@ -26,9 +26,9 @@
             sage: P.cardinality()
             1
             sage: P.cardinality(-1) # This test breaks
-            -1
+            1
         """
-        return n
+        return 1 # we don't want to change the semantics of cardinality()
 ```
 
 This seems to be caused by not checking if the parent class has a `cardinality()` function implemented, and just overriding it with `_cardinality_from_list()` (which takes no [optional] parameters).

[tscrim]

comment:1

Tweaked example to show we don't want to change the semantics of cardinality().

[tscrim]

Author: Travis Scrimshaw

[tscrim]

comment:2

Patch which has _cardinality_from_* take dummy optional args and discard them.

[tscrim]

Description changed:

--- 
+++ 
@@ -33,3 +33,4 @@
 
 This seems to be caused by not checking if the parent class has a `cardinality()` function implemented, and just overriding it with `_cardinality_from_list()` (which takes no [optional] parameters).
 
+NOTE: The failure will **only** show up while running doctests using `sage -t`, not while running sage.

[AndrewMathas]

Changed keywords from finite sets, cardinality to finite sets, cardinality, days45

[AndrewMathas]

Reviewer: Andrew Mathas

[AndrewMathas]

comment:4

The patch is quite innocuous and fixes a problem. All doc tests pass.

Can you add a doctest for testing for this? Presumably it showed up in a real class. Happy for you to set this to a positive review once that's done.