Generalize MapCombinatorialClass to ImageSubobject/ImageSet, add method Map.image

[Matthias Köppe]

EnumeratedSets.ParentMethods.map uses MapCombinatorialClass.

map's docstring explains that that class needs to be refactored to use categories.

Part of #12913 (Meta-ticket: Deprecate CombinatorialClass in favor of the EnumeratedSet's categories)

See also:

• ​https://trac.sagemath.org/ticket/31653#comment:1 for a discussion of existing image and pushforward methods of maps.

• #24176

[git]

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

​44f0a31	src/sage/combinat/combinat.py: Update copyright according to git blame -w --date=format:%Y src/sage/combinat/combinat.py | sort -k2
​fef2e03	sage.sets.image_set: New

[git]

Branch pushed to git repo; I updated commit sha1. Last 10 new commits:

​b67092c	Set_base.union, intersection, difference, symmetric_difference: Convert to Set if necessary
​e144389	ConvexSet_base.intersection: Remove abstract method; now inherits Set_base.intersection
​bc26d4f	{LatticePolytopeClass, ConvexRationalPolyhedralCone._some_elements_: New
​e768463	Merge #31877
​cb93c99	RealSet: Inherit from Set_base, Set_boolean_operators, Set_add_sub_operators
​3a6f9bd	RealSet.symmetric_difference: New
​08c52c2	Set_base._test_as_set_object: Skip _test_pickling
​200b1ef	Merge tag '9.4.beta4' into t/32013/initialize_a_set_from_a_convexset_base_instance
​6a880c0	Merge #32013
​8e920e9	Make MapCombinatorialClass a subclass of ImageSubobject

[git]

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

​b57c6a5

ImageSubobject: Handle is_injective, fix repr

[git]

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

​5223859

ImageSubobject: Fixup finite/infinite

[Matthias Köppe]

Here's an attempt

[git]

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

​2d0bd77

Map.image: Fixup

[git]

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

​7d36212

ImageSubobject: Handle CallableSymbolicExpression as map

[git]

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

​d9cff76	Function._sympy_, SympyConverter.__call__: New
​17bc9b4	Merge #32130
​f80d5f4	PoorManMap, ImageSubobject: Add _sympy_ method

[Matthias Köppe]

Help with extending the doctests with examples relevant for combinatorics and algebra would be very welcome.

[git]

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

​63c2cfe	ConvexSet_base._test_convex_set: Do not test _test_as_set_object here
​55240bb	Merge #30473
​fff2a79	src/sage/sets/set.py: Fix docstring markup
​e433953	Merge #32013
​84eec2a	Merge tag '9.4.beta5' into t/32121/replace_mapcombinatorialclass__add_methods_map_image__map_pushforward

[Travis Scrimshaw]

Sorry for being a bit slow to get to this.

This class is not so widely used within the codebase. I can only find:

• combinat/set_partition_ordered.py: fatter()
• combinat/root_systems/root_lattice_realizations.py: negative_roots()
• combinat/composition.py: finer() and fatter()
• combinat/free_module.py: The indices for the tensor product of CFMs

So if it works fine for these files, then it is a full replacement. These also are fairly extensively tested already I think. Thus, I don't think new tests are necessarily needed.

[git]

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

​de96639	src/sage/combinat/combinat.py: Update copyright according to git blame -w --date=format:%Y src/sage/combinat/combinat.py | sort -k2
​4a0d7dd	Map.image, sage.sets.image_set: New
​61b9b7e	Make MapCombinatorialClass a subclass of ImageSubobject
​6e0569e	ImageSubobject: Handle is_injective, fix repr
​065a1fc	ImageSubobject: Fixup finite/infinite
​61a8a48	ImageSubobject: Handle CallableSymbolicExpression as map
​9e97ea0	PoorManMap, ImageSubobject: Add _sympy_ method

[Matthias Köppe]

Rebased, partially squashed, on 9.4.rc0

[Travis Scrimshaw]

A few trivial failures, but also a more deeper issue:

sage -t --long --random-seed=0 src/sage/categories/finite_dimensional_modules_with_basis.py  # 1 doctest failed
sage -t --long --random-seed=0 src/sage/combinat/free_module.py  # 1 doctest failed
sage -t --long --random-seed=0 src/sage/categories/enumerated_sets.py  # 2 doctests failed

Additionally, there are many places that need some doctests. Would you like me to add them?

[Matthias Köppe]

Any help on the ticket is welcome! I won't be able to work on it before mid August

[Travis Scrimshaw]

I finally got around to working on this. I have made some tweaks, and I also fixed a few bugs. The biggest was moving the generic image() definition to the category. This was conflicting with the image defined for ModulesWithBasis(). There are still a few things that need doctests, but I don't know how to effectively doctest them (other than with something trivial).

---

New commits:

​8d87ea5	Merge branch 'u/mkoeppe/replace_mapcombinatorialclass__add_methods_map_image__map_pushforward' of git://trac.sagemath.org/sage into public/combinat/image_sets-32121
​a7877ce	Initial round of fixing issues with ImageSet.
​5beba14	Fixing remaining things with ImageSet and adding some doctests.

[git]

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

​3197cd3

Merge branch 'develop' into public/combinat/image_sets-32121

[git]

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

​b845bfb

Merge branch 'develop' into public/combinat/image_sets-32121

[git]

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

​7c091af

Merge tag '9.6.beta1' into t/32121/public/combinat/image_sets-32121

[git]

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

​0d850f1

src/sage/categories/sets_cat.py: Update copyright according to git blame -w --date=format:%Y src/sage/categories/sets_cat.py | sort -k2

[git]

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

​0b70dc9

src/sage/sets/image_set.py: Small doc edits

[git]

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

​8688280	WIP
​f8d559a	PoorManMap._sympy_: Add doctest

[git]

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

​6605ad2

src/sage/sets/image_set.py: Small doc edit

[git]

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

​4bff6c5

MapCombinatorialClass: Remove methods now inherited from ImageSubobject, restore public slots cc, f

[git]

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

​7c6aacf

EnumeratedSets.map, CombinatorialClass.map, MapCombinatorialClass: New keyword argument is_injective

[git]

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

​772970d

src/sage/categories/enumerated_sets.py: Remove outdated warning

[Matthias Köppe]

I think this is ready now

[git]

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

​bbc2f02

src/sage/combinat/free_module.py: Update doctest output

[Travis Scrimshaw]

Thank you. Just a few more little things:

• Since _star is only called in one place, I think it should just be done inline. This will also save you from having to write a docstring/test for it.
• lift and retract need doctests.
• MapCombinatorialClass, _an_element_, and ImageSet are missing periods at the end of their one-line descriptions.
• In both map() docs:

  -        Use ``is_injective=False`` to get a correct result in this case.
  +        Use ``is_injective=False`` to get a correct result in this case::
   
               sage: P.map(len, is_injective=False).list()
               [1, 2, 3, 4]
  

           INPUT:
   
           - ``is_injective`` -- boolean (default: ``True``) whether to assume
  -          that ``f`` is injective.
  +          that ``f`` is injective
  

• I am not sure how I feel about being so detailed in the code copyright declaration. I understand the legal reason, but it is a bit tedious to maintain (it is already out of date) and might create legal issues with the license (including some people may not be claiming copyright as they did not change that statement). This is probably more towards bikeshedding though...

[Matthias Köppe]

Replying to tscrim:

I am not sure how I feel about being so detailed in the code copyright declaration. I understand the legal reason,

More importantly perhaps, it looks much better if there is year for year activity by several authors. If copyright notices stop in 2006 because the updating work has been neglected, the file and perhaps the whole project looks stale to the casual observer.

it is a bit tedious to maintain (it is already out of date)

It does not have to be perfect. And if it looks updated, people will be motivated to keep it updated.

and might create legal issues with the license (including some people may not be claiming copyright as they did not change that statement).

Copyright is established whether there is intent or not, whenever there is a copyrightable change in a given year.

[git]

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

​d3a90ee

src/sage/sets/image_set.py: Fold ImageSubobject._star into __init__

[git]

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

​2894aa8	MapCombinatorialClass: Fix one-line description
​b301b59	src/sage/sets/image_set.py: Fix one-line descriptions
​ed47743	src/sage/categories/enumerated_sets.py, src/sage/combinat/combinat.py: Fix doc markup
​0e9431c	ImageSubobject.{lift,retract}: Add doctests

[Travis Scrimshaw]

Thank you. Green bot => positive review.

[Matthias Köppe]

Patchbot failures are unrelated.

[Volker Braun]

This doctest seems to be random

**********************************************************************
File "src/sage/sets/image_set.py", line 275, in sage.sets.image_set.ImageSet
Failed example:
    ImageSet(sos, Set([(3, 4), (3, -4)]))
Expected:
    Image of
     {(3, -4), (3, 4)} by
     The map (x, y) |--> x^2 + y^2 from Vector space of dimension 2 over Symbolic Ring
Got:
    Image of {(3, 4), (3, -4)} by The map (x, y) |--> x^2 + y^2 from Vector space of dimension 2 over Symbolic Ring
**********************************************************************
1 item had failures:
   1 of   9 in sage.sets.image_set.ImageSet
    [52 tests, 1 failure, 0.25 s]
----------------------------------------------------------------------
sage -t --long --random-seed=0 src/sage/sets/image_set.py  # 1 doctest failed
----------------------------------------------------------------------

[git]

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

​038e354

src/sage/sets/image_set.py: Adjust doctest output to unspecified print order of Sets