Opened 22 months ago
Closed 13 months ago
#30832 closed enhancement (fixed)
Connect RealSet to sage.manifolds
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage9.4 
Component:  manifolds  Keywords:  
Cc:  ghmjungmath, egourgoulhon, tscrim  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Michael Jung 
Report Upstream:  N/A  Work issues:  
Branch:  81b43f4 (Commits, GitHub, GitLab)  Commit:  81b43f4bb4d337f3cb1cf59c2d866fddab51b6b2 
Dependencies:  #31644, #31877  Stopgaps: 
Description (last modified by )
Instances of RealSet
that are finite unions of open intervals should be connected in some way to sage.manifolds
, in particular to OpenInterval
and RealLine
(which are both in the global namespace).
In this ticket, we add functionality to initialize a RealSet
from instances of OpenInterval
and RealLine
, as well as from their topological closures (#31644)
Change History (26)
comment:1 Changed 19 months ago by
 Cc tscrim added
comment:2 Changed 18 months ago by
 Milestone changed from sage9.3 to sage9.4
comment:3 Changed 15 months ago by
 Dependencies set to #31644
 Description modified (diff)
comment:4 Changed 15 months ago by
 Branch set to u/mkoeppe/connect_realset_to_sage_manifolds
comment:5 Changed 15 months ago by
 Commit set to fb33c0f4f26e907ba58a75224efaf263f245d69d
 Status changed from new to needs_review
Last 10 new commits:
e7f7a7d  Merge #31764

9259a2c  Merge #31798

ac53984  src/sage/manifolds/subsets/__init__.py: New

21739de  src/sage/manifolds/subset.py: Simplify code, make pyflakes happy

6bbd4ae  src/sage/manifolds/subset.py: Add coding header

2d7fda7  src/sage/manifolds/subset.py: Simplify code more to make pyflakes happy

8d3ad85  Merge #31764

60505b0  Merge #31798

9dd9f90  Merge #31644

fb33c0f  RealSet: Allow initialization from RealLine and OpenInterval instances and OpenInterval closures

comment:6 Changed 15 months ago by
Restricted to open intervals (not closures), this is nothing but the forgetful functor from manifolds into sets. Perhaps it is a good idea to add this functor somehow?
Other than that it looks good! Let's wait for the patchbot.
comment:7 followup: ↓ 19 Changed 15 months ago by
... or rather from manifolds to topological spaces (see #31877).
Do we have forgetful functors anywhere already?
comment:8 followup: ↓ 11 Changed 15 months ago by
We may also want to consider
 adding a method
RealSet.as_manifold_subset
 extending the
RealSet
constructors (default constructor andRealSet.open
etc.) so that they can build manifold objects; for example ifstructure
orname
is passed; then perhaps the global bindingsRealLine
andOpenInterval
can be deprecated.
comment:9 followup: ↓ 12 Changed 15 months ago by
Notice that RealSet
supports unions of intervals whereas OpenInterval
does not.
comment:10 followup: ↓ 13 Changed 15 months ago by
Moreover, you represented closed or clopen intervals by a topological closure, which is fine for now. But as soon as we have manifolds with boundary, we probably want them to be represented by those objects/instances instead (or as well).
comment:11 in reply to: ↑ 8 Changed 15 months ago by
 Reviewers set to Michael Jung
 Status changed from needs_review to positive_review
LGTM
comment:12 in reply to: ↑ 9 ; followup: ↓ 14 Changed 15 months ago by
Replying to ghmjungmath:
Notice that
RealSet
supports unions of intervals whereasOpenInterval
does not.
Yes, it does. You can initialize a RealSet
using several OpenInterval
instances, and it will take the union.
comment:13 in reply to: ↑ 10 Changed 15 months ago by
Thanks for the review!
comment:14 in reply to: ↑ 12 ; followup: ↓ 15 Changed 15 months ago by
Replying to mkoeppe:
Replying to ghmjungmath:
Notice that
RealSet
supports unions of intervals whereasOpenInterval
does not.Yes, it does. You can initialize a
RealSet
using severalOpenInterval
instances, and it will take the union.
That is not what I mean. There are no unions of OpenInterval
on the manifold level yet. This is, for example, important for a potential RealSet.as_manifold_subset
.
comment:15 in reply to: ↑ 14 Changed 15 months ago by
Replying to ghmjungmath:
There are no unions of
OpenInterval
on the manifold level yet. This is, for example, important for a potentialRealSet.as_manifold_subset
.
If you create OpenInterval
instances with the same ambient_interval
set, then you can take unions.
sage: I02 = OpenInterval(0, 2, ambient_interval=RealLine()) sage: I13 = OpenInterval(1, 3, ambient_interval=RealLine()) sage: I02.union(I13) Open subset (0, 2)_union_(1, 3) of the Real number line R
comment:16 Changed 15 months ago by
Right, but
sage: I02 = OpenInterval(0, 2, ambient_interval=RealLine()) sage: I13 = OpenInterval(1, 3, ambient_interval=RealLine()) sage: U = I02.union(I13) sage: RealSet(U)
won't work, no?
comment:17 Changed 15 months ago by
Good catch...
comment:18 Changed 15 months ago by
Folllow up in #31878.
comment:19 in reply to: ↑ 7 Changed 15 months ago by
comment:20 Changed 14 months ago by
 Commit changed from fb33c0f4f26e907ba58a75224efaf263f245d69d to 785aed0f2ef1efb2282e003c39be671ebcd74fd9
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. Last 10 new commits:
fd4506a  Merge #31732

7fed9ef  Merge #31736

c4acd09  Merge tag '9.4.beta2' into t/31763/manifoldsubset__new_methods_declare_subset__declare_superset

2ba18ab  Merge #31763

0e9e5cd  Merge #31732

a96f736  Merge #31736

582b58d  Merge #31763

2113f78  Merge #31764

9abc617  Merge #31798

785aed0  Merge #31644

comment:21 Changed 14 months ago by
 Status changed from needs_review to positive_review
Trivial merge with updated dependency
comment:22 Changed 14 months ago by
 Dependencies changed from #31644 to #31644, #31877
comment:23 Changed 14 months ago by
 Commit changed from 785aed0f2ef1efb2282e003c39be671ebcd74fd9 to 81b43f4bb4d337f3cb1cf59c2d866fddab51b6b2
 Status changed from positive_review to needs_review
Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:
4b09685  RealSet: Put it in a suitable subcategory of TopologicalSpaces()

46eed0e  RealSet.ambient: Change to a normal method

5b8ceca  InternalRealInterval, RealSet: Add _latex_ methods

69ca854  RealSet._repr_: Use unicode cup sign instead of +

dbdfc06  InternalRealInterval, RealSet: Remove extra whitespace in latex, add documentation

7f56338  PiecewiseFunction: Adjust doctests for changed RealSet repr

8abdc8b  src/sage/functions/piecewise.py: Add coding header

5b0f85d  Merge #31880

81b43f4  Merge branch 't/31877/refine_category_of_realset' into t/30832/connect_realset_to_sage_manifolds

comment:24 Changed 14 months ago by
Another trivial merge to remove a merge conflict
comment:25 Changed 14 months ago by
 Status changed from needs_review to positive_review
comment:26 Changed 13 months ago by
 Branch changed from u/mkoeppe/connect_realset_to_sage_manifolds to 81b43f4bb4d337f3cb1cf59c2d866fddab51b6b2
 Resolution set to fixed
 Status changed from positive_review to closed
Setting new milestone based on a cursory review of ticket status, priority, and last modification date.