ManifoldSubset: Change some methods to generators

[mkoeppe]

This ticket proposes to change some methods that currently return lists to generators - like the method open_supersets added in #31677. This is in line with the changes in the Python standard library when we moved from Python 2 to 3.

ManifoldSubset:

• open_covers() currently returns a list of lists

  • change to generator of ManifoldSubsetFiniteFamily instances
  • add optional argument trivial to simplify the common use case that only needs the nontrivial open covers

• subsets() currently returns a frozenset

  • change to generator of ManifoldSubset instances

These API changes will probably make some updates to sage.manifolds worksheets that are maintained outside of the Sage tree necessary.

Follow-up for some methods of Manifold in #31720.

Depends on #31680

CC: @mjungmath @egourgoulhon @tscrim

Component: manifolds

Author: Matthias Koeppe

Branch/Commit: e026e7a

Reviewer: Eric Gourgoulhon

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

[mkoeppe]

Branch: u/mkoeppe/manifoldsubsetmanifoldchange_some_methods_to_generators

[mkoeppe]

Description changed:

--- 
+++ 
@@ -2,7 +2,7 @@

 `ManifoldSubset`:
 - `open_covers()` currently returns a list of lists
-  - change to generator of `FiniteManifoldSubsetFamily` instances 
+  - change to generator of `ManifoldSubsetFiniteFamily` instances 

 - `subsets()` currently returns a `frozenset`
   - change to generator of `ManifoldSubset` instances

[mkoeppe]

New commits:

186707b

ManifoldSubset.subsets: Change to generator

[mkoeppe]

Commit: 186707b

[mkoeppe]

Description changed:

--- 
+++ 
@@ -3,6 +3,7 @@
 `ManifoldSubset`:
 - `open_covers()` currently returns a list of lists
   - change to generator of `ManifoldSubsetFiniteFamily` instances 
+  - add optional argument `trivial` to simplify the common use case that only needs nontrivial open covers

 - `subsets()` currently returns a `frozenset`
   - change to generator of `ManifoldSubset` instances

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 186707b to 78cc27a

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

42abda7	Adding `__bool__` for other families.
1f21a8f	Merge #31717
f095988	FiniteManifoldObjectFamily.__bool__: Remove, inherited from superclass after #31717
5d87cee	Manifold{Object,Subset}FiniteFamily: Rename from FiniteManifold{Object,Subset}Family
2f2ace2	src/doc/en/reference/manifolds/manifold.rst: Add sage.manifolds.family
1aff58a	Fix up docstring markup
b922066	ManifoldSubsetFiniteFamily: If all subsets are open, include 'open' in repr
30271af	Fixup doctest
adac07a	Merge #31680
78cc27a	ManifoldSubset.open_covers: Change to generator, add optional arg 'trivial'; update uses

[mkoeppe]

Author: Matthias Koeppe

[mkoeppe]

Description changed:

--- 
+++ 
@@ -3,14 +3,10 @@
 `ManifoldSubset`:
 - `open_covers()` currently returns a list of lists
   - change to generator of `ManifoldSubsetFiniteFamily` instances 
-  - add optional argument `trivial` to simplify the common use case that only needs nontrivial open covers
+  - add optional argument `trivial` to simplify the common use case that only needs the nontrivial open covers

 - `subsets()` currently returns a `frozenset`
   - change to generator of `ManifoldSubset` instances

-`Manifold`:
-- `top_charts()` currently returns a list
-  - change to generator of charts
-- `coord_changes()` currently returns a copy
-  - change to generator of pairs `((from, to), map)` ... so that `dict(M.coord_changes())` will create the copy
-     
+Follow-up for some methods of `Manifold` in #31720.
+

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

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

e026e7a

ManifoldSubset.subset_digraph: Use open_covers method

[7ed8c4ca-6d56-4ae9-953a-41e42b4ed313]

Changed commit from 78cc27a to e026e7a

[mkoeppe]

Description changed:

--- 
+++ 
@@ -8,5 +8,8 @@
 - `subsets()` currently returns a `frozenset`
   - change to generator of `ManifoldSubset` instances

+These API changes will probably make some updates to sage.manifolds worksheets that are maintained outside of the Sage tree necessary.
+
+
 Follow-up for some methods of `Manifold` in #31720.

[egourgoulhon]

Reviewer: Eric Gourgoulhon

[egourgoulhon]

comment:8

LGTM. Thanks!

[mkoeppe]

comment:9

Thanks for reviewing!

[mjungmath]

comment:10

I assume it has a purpose, but just out of curiosity: why do you make a difference between families of manifold objects and families of manifold subsets? Do you have a further usage in mind other than just subsets? Because all the examples you provide are still with subsets.

[mjungmath]

comment:11

I can imagine this can be useful for frames whose domains cover the manifold? Perhaps this is a more suitable example for the non-subset case to show the difference?

Some time ago I provided a helper function _get_min_covering (can be found in manifolds/manifold.py) to obtain a minimal amount of manifold objects necessary to cover the manifold. It looks like this method is more suited within the family class you just provided.

Another example in the field I could imagine is for orientations. An orientation is given by a family of charts/frames, too, but yet not implemented as such.

[mkoeppe]

comment:12

Replying to @mjungmath:

I assume it has a purpose, but just out of curiosity: why do you make a difference between families of manifold objects and families of manifold subsets? Do you have a further usage in mind other than just subsets?

Yes, in #31732 I use ManifoldObjectFiniteFamily directly for the family of open covers. Its elements are ManifoldSubsetFiniteFamily instances.

I agree that charts/frames are likely to benefit from becoming finite families too. This will also need a separate subclass because they are indexed not by names but by coordinate tuples.

[mjungmath]

comment:13

Replying to @mkoeppe:

Replying to @mjungmath:

I assume it has a purpose, but just out of curiosity: why do you make a difference between families of manifold objects and families of manifold subsets? Do you have a further usage in mind other than just subsets?

Yes, in #31732 I use ManifoldObjectFiniteFamily directly for the family of open covers. Its elements are ManifoldSubsetFiniteFamily instances.

Alright, that makes sense. Just a personal taste: I think its better to add some distinct examples to ManifoldObjectFiniteFamily to clarify the difference to ManifoldSubsetFiniteFamily.

I agree that charts/frames are likely to benefit from becoming finite families too. This will also need a separate subclass because they are indexed not by names but by coordinate tuples.

Right. But don't you think it should rather be a new common parent class both inherit from?

In any case, this is something definitely need, indeed!

[mkoeppe]

comment:14

Replying to @mjungmath:

I think its better to add some distinct examples to ManifoldObjectFiniteFamily to clarify the difference to ManifoldSubsetFiniteFamily.

Sure, let's do that when we introduce some more applications to families.

Replying to @mkoeppe:

I agree that charts/frames are likely to benefit from becoming finite families too. This will also need a separate subclass because they are indexed not by names but by coordinate tuples.

Right. But don't you think it should rather be a new common parent class both inherit from?

Sure, that makes sense.

In any case, this is something definitely need, indeed!

Let's take this discussion to #31720 (Manifold: Change some methods to generators), which introduces generators for charts. Families could be introduced in the same ticket -- but I will need some guidance there what the keys should be.

[vbraun]

Changed branch from u/mkoeppe/manifoldsubsetmanifoldchange_some_methods_to_generators to e026e7a