Set_object_enumerated should be in FiniteEnumeratedSets()

[mkoeppe]

sage: Set([0, 1, 2, 3])
{0, 1, 2, 3}
sage: _.category()
Category of finite sets
sage: Set([0, 1, 2, 3], category=FiniteEnumeratedSets())
TypeError: Set() got an unexpected keyword argument 'category'

Likewise for the Set_object_binary classes (which implement lazy union, intersection, ...), the category should be computed from the categories of the inputs.

Also we add a category argument to the Set factory and the __init__ methods of the various implementation classes.

Follow ups:

• Deprecate FiniteEnumeratedSet in the global namespace, add method is_subset to Sets.ParentMethods #34398 the relation between Set_object_enumerated and FiniteEnumeratedSet (and TrivialFamily, EnumeratedFamily) should be clarified
• Set() should construct facade parents or use ElementWrapper #34387 Set() should construct facade parents

CC: @tscrim

Component: categories

Author: Matthias Koeppe

Branch/Commit: e58d2ab

Reviewer: Travis Scrimshaw

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

[mkoeppe]

Branch: u/mkoeppe/set_object_enumerated_should_be_in_finiteenumeratedsets__

[mkoeppe]

Commit: b19a1ff

[mkoeppe]

New commits:

b19a1ff

src/sage/sets/set.py: Accept and handle 'category' arguments

[mkoeppe]

Author: Matthias Koeppe

[sagetrac-git]

Changed commit from b19a1ff to d461411

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

d461411

Update doctest outputs

[sagetrac-git]

Changed commit from d461411 to df327b9

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

df327b9

src/sage/sets/set.py: Refine categories of unions, intersections, differences

[tscrim]

comment:8

You shouldn't need the cardinality() method as that should be given by the category.

You can simplify these checks:

-        if all(S in Sets().Finite() for S in (X, Y)):
-            category = category.Finite()
         if all(S in Sets().Enumerated() for S in (X, Y)):
             category = category.Enumerated()
         if any(S in Sets().Infinite() for S in (X, Y)):
             category = category.Infinite()
+        elif all(S in Sets().Finite() for S in (X, Y)):
+            category = category.Finite()
         Set_object_binary.__init__(self, X, Y, "union", "\\cup", category=category)

-        if X in Sets().Finite():
-            category = category.Finite()
         if X in Sets().Enumerated():
             category = category.Enumerated()
+        if X in Sets().Finite():
+            category = category.Finite()
+        elif X in Sets().Infinite() and Y in Sets().Finite():
-        if X in Sets().Infinite() and Y in Sets().Finite():
             category = category.Infinite()
         Set_object_binary.__init__(self, X, Y, "difference", "-", category=category)

I am also slightly worried about this change:

diff --git a/src/sage/categories/enumerated_sets.py b/src/sage/categories/enumerated_sets.py
index 563a802..e1dcbd8 100644
--- a/src/sage/categories/enumerated_sets.py
+++ b/src/sage/categories/enumerated_sets.py
@@ -137,7 +137,7 @@ class EnumeratedSets(CategoryWithAxiom):
             sage: S = EnumeratedSets()(Set([1, 2, 3])); S
             {1, 2, 3}
             sage: S.category()
-            Category of facade finite enumerated sets
+            Category of finite enumerated sets
 
         Also Python3 range are now accepted::
 

Can you explain why this happened? I can't see the reason immediately from your changes.

[mkoeppe]

comment:9

Thanks! I agree with these simplifications.

I'll investigate how the facade got lost

[mkoeppe]

comment:10

By the way, should these Set_object parents all become facade parents?

[tscrim]

comment:11

I think so. All of the various constructions (e.g., __iter__, list, etc.) all return objects whose parent is not the Set.

[mkoeppe]

comment:12

Replying to @tscrim:

I am also slightly worried about this change:

diff --git a/src/sage/categories/enumerated_sets.py b/src/sage/categories/enumerated_sets.py
index 563a802..e1dcbd8 100644
--- a/src/sage/categories/enumerated_sets.py
+++ b/src/sage/categories/enumerated_sets.py
@@ -137,7 +137,7 @@ class EnumeratedSets(CategoryWithAxiom):
             sage: S = EnumeratedSets()(Set([1, 2, 3])); S
             {1, 2, 3}
             sage: S.category()
-            Category of facade finite enumerated sets
+            Category of finite enumerated sets
 
         Also Python3 range are now accepted::
 

Can you explain why this happened?

Set([1, 2, 3]) is already in EnumeratedSets(), so passing EnumeratedSets._call_ does not have to construct another wrapper.

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

8a30493

src/sage/sets/set.py: Simplify category checks as suggested

[sagetrac-git]

Changed commit from df327b9 to 8a30493

[mkoeppe]

comment:14

Replying to @tscrim:

should these Set_object parents all become facade parents?

I think so. All of the various constructions (e.g., __iter__, list, etc.) all return objects whose parent is not the Set.

I'll do this in #34387

[sagetrac-git]

Changed commit from 8a30493 to 8aa274e

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

141ebf1	src/sage/categories/sets_cat.py: Update doctest output
8aa274e	src/sage/categories/sets_cat.py: Fix doctest

[mkoeppe]

comment:17

Replying to @tscrim:

You shouldn't need the cardinality() method as that should be given by the category.

The new method Set_object_intersection.cardinality needs to override Set_object.cardinality.

Now why exactly Set_object_binary subclasses Set_object is another question but not one for this ticket

[sagetrac-git]

Changed commit from 8aa274e to d711a61

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

d711a61

src/sage/structure/parent.pyx: Update doctest output

[tscrim]

comment:19

Replying to @mkoeppe:

Replying to @tscrim:

You shouldn't need the cardinality() method as that should be given by the category.

The new method Set_object_intersection.cardinality needs to override Set_object.cardinality.

Ah, because of Set_object. Then we should change things in the base class. Perhaps Set_object.is_finite() should also test the category?

[mkoeppe]

comment:20

Good idea.

[sagetrac-git]

Changed commit from d711a61 to e58d2ab

[sagetrac-git]

Branch pushed to git repo; I updated commit sha1. New commits:

e63e4f0	Set_object.is_finite: Check category
970d467	Set_object.cardinality: Check category
e58d2ab	Set_object_intersection.cardinality: Remove; move delegation to super to Set_object

[tscrim]

comment:22

This ended up being a good test:

sage: X = S.difference(T); X
Set-theoretic difference of Set of elements of Rational Field and Set of elements of Integer Ring
sage: X.category()
Category of sets

because QQ does have a (canonical) enumeration that isn't recognized by Sage:

sage: QQ in EnumeratedSets()
False

However, fixing that is for another ticket.

[tscrim]

Reviewer: Travis Scrimshaw

[mkoeppe]

comment:23

Yes, I was briefly surprised by this one.

[mkoeppe]

comment:24

By the way, I'm a bit annoyed by EnumeratedSets.ParentMethods.__getitem__ (which is the same as unrank). Do you think people use that at all?

[tscrim]

comment:25

It is quite possible that people in combinatorics use it. For example, things like

sage: ASM = AlternatingSignMatrices(4)
sage: ASM[1:3]
[
[0 1 0 0]  [1 0 0 0]
[1 0 0 0]  [0 0 1 0]
[0 0 1 0]  [0 1 0 0]
[0 0 0 1], [0 0 0 1]
]

should call it (the implementation does a redirect to the category).

[mkoeppe]

comment:26

There's probably a better solution for getting this kind of syntax when it's convenient, maybe using a mixin class (like the mixin classes Set_boolean_operators etc.). Having it in the category feels a bit misplaced.

Also one could maybe have something like an as_sequence method that would return a SequenceView (and thus give [] and slicing), see #32100

But that's for another ticket. Thanks for the review!

[tscrim]

comment:28

Replying to @mkoeppe:

There's probably a better solution for getting this kind of syntax when it's convenient, maybe using a mixin class (like the mixin classes Set_boolean_operators etc.). Having it in the category feels a bit misplaced.

That would negate a fair part of the category framework, it removes the fact that an enumerated set should (generally) behave like a list, and essentially every EnumeratedSet would have this proposed mixin class (thus being a code smell for why isn't in the category).

But that's for another ticket. Thanks for the review!

Indeed. No problem.

[vbraun]

Changed branch from u/mkoeppe/set_object_enumerated_should_be_in_finiteenumeratedsets__ to e58d2ab