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: | sage-9.4 |
Component: | manifolds | Keywords: | |
Cc: | gh-mjungmath, 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 sage-9.3 to sage-9.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 follow-up: ↓ 19 Changed 15 months ago by
... or rather from manifolds to topological spaces (see #31877).
Do we have forgetful functors anywhere already?
comment:8 follow-up: ↓ 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 follow-up: ↓ 12 Changed 15 months ago by
Notice that RealSet
supports unions of intervals whereas OpenInterval
does not.
comment:10 follow-up: ↓ 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 ; follow-up: ↓ 14 Changed 15 months ago by
Replying to gh-mjungmath:
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 ; follow-up: ↓ 15 Changed 15 months ago by
Replying to mkoeppe:
Replying to gh-mjungmath:
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 gh-mjungmath:
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.