Opened 14 months ago
Closed 13 months ago
#32013 closed enhancement (fixed)
Initialize a Set from a ConvexSet_base instance
Reported by:  mkoeppe  Owned by:  

Priority:  major  Milestone:  sage9.4 
Component:  geometry  Keywords:  
Cc:  ghkliem, tscrim  Merged in:  
Authors:  Matthias Koeppe  Reviewers:  Travis Scrimshaw 
Report Upstream:  N/A  Work issues:  
Branch:  fff2a79 (Commits, GitHub, GitLab)  Commit:  fff2a79b5936a2b354d80dca78612eee5301a701 
Dependencies:  #30473  Stopgaps: 
Description (last modified by )
A Polyhedron
is not a Parent
, so Set
refuses to construct the set of its elements.
We change this by creating a new abstract base class for nonnecessarilyparent sets with methods union
, intersection
, etc.
ConvexSet_base
(from #31919) and RealSet
now both inherit from Set_base
. To complete the implementation of the Set_base
protocol, we add an implementation of RealSet.symmetric_difference
.
So we can now do the following things:
sage: Set(polytopes.cube()) Set of elements of A 3dimensional polyhedron in ZZ^3 defined as the convex hull of 8 vertices sage: polytopes.cube().union(polytopes.tetrahedron()) Settheoretic union of Set of elements of A 3dimensional polyhedron in ZZ^3 defined as the convex hull of 8 vertices and Set of elements of A 3dimensional polyhedron in ZZ^3 defined as the convex hull of 4 vertices
Change History (33)
comment:1 Changed 14 months ago by
 Branch set to u/mkoeppe/initialize_a_set_from_a_convexset_base_instance
comment:2 Changed 14 months ago by
 Cc ghkliem tscrim added
 Commit set to e1443896851ac805a89e8747e8c7412bbc7d9d10
 Dependencies set to #31990
 Description modified (diff)
 Status changed from new to needs_review
comment:3 Changed 14 months ago by
 Commit changed from e1443896851ac805a89e8747e8c7412bbc7d9d10 to bc26d4f1dfef8d64ffb60db717a2db7a155771d6
Branch pushed to git repo; I updated commit sha1. New commits:
bc26d4f  {LatticePolytopeClass, ConvexRationalPolyhedralCone._some_elements_: New

comment:4 Changed 14 months ago by
 Dependencies changed from #31990 to #31990, #32025
comment:5 Changed 14 months ago by
 Dependencies changed from #31990, #32025 to #31990, #31877, #32025
comment:6 Changed 14 months ago by
 Commit changed from bc26d4f1dfef8d64ffb60db717a2db7a155771d6 to afad76f41652e85f9c1ed34504ca9e4b08e355f5
Branch pushed to git repo; I updated commit sha1. Last 10 new commits:
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

e768463  Merge #31877

cb93c99  RealSet: Inherit from Set_base, Set_boolean_operators, Set_add_sub_operators

3a6f9bd  RealSet.symmetric_difference: New

afad76f  sage.sets.set.Set_object: Add _sympy_ methods to subclasses

comment:7 Changed 14 months ago by
 Commit changed from afad76f41652e85f9c1ed34504ca9e4b08e355f5 to 34d920be955ff91868e0c144444af8fc819493fa
Branch pushed to git repo; I updated commit sha1. New commits:
34d920b  Set_base._test_as_set_object: Skip _test_pickling

comment:8 Changed 14 months ago by
 Dependencies changed from #31990, #31877, #32025 to #31990, #31877
comment:9 Changed 14 months ago by
 Commit changed from 34d920be955ff91868e0c144444af8fc819493fa to 08c52c2535aeb4c2eadc43e22979df076a6d0760
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
08c52c2  Set_base._test_as_set_object: Skip _test_pickling

comment:10 Changed 14 months ago by
 Description modified (diff)
comment:11 Changed 13 months ago by
 Commit changed from 08c52c2535aeb4c2eadc43e22979df076a6d0760 to 200b1ef2779ae14db90d97cfc94ff5ffb2b8710b
Branch pushed to git repo; I updated commit sha1. New commits:
200b1ef  Merge tag '9.4.beta4' into t/32013/initialize_a_set_from_a_convexset_base_instance

comment:12 Changed 13 months ago by
 Reviewers set to Travis Scrimshaw
 Status changed from needs_review to positive_review
LGTM.
comment:13 Changed 13 months ago by
Thanks!
comment:14 Changed 13 months ago by
 Status changed from positive_review to needs_work
Sorry, I was a bit premature. I am getting doctest failures:
sage t randomseed=0 src/sage/geometry/convex_set.py # 2 doctests failed
File "src/sage/geometry/convex_set.py", line 444, in sage.geometry.convex_set.ConvexSet_base._test_convex_set Failed example: TestSuite(BiggerOnTheInside()).run(skip=('_test_pickling', '_test_contains')) Expected: Failure in _test_convex_set: ... The following tests failed: _test_convex_set Got: Failure in _test_an_element:
Just needs to update the expected output.
comment:15 Changed 13 months ago by
Thanks for catching this, I'll take a look tomorrow.
comment:16 Changed 13 months ago by
 Commit changed from 200b1ef2779ae14db90d97cfc94ff5ffb2b8710b to 63c2cfee88ed2ded272139fd48733fdd6fab0526
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

comment:17 Changed 13 months ago by
 Status changed from needs_work to needs_review
Found and cherrypicked the fix for it from another ticket.
comment:18 Changed 13 months ago by
 Dependencies #31990, #31877 deleted
comment:20 Changed 13 months ago by
Thanks!
comment:21 Changed 13 months ago by
 Commit changed from 63c2cfee88ed2ded272139fd48733fdd6fab0526 to 55240bb9b6e4db768a2102b4065752e968a6fa18
 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:
da8893f  Use sage.typeset.unicode_characters in TensorProductFunctor and SignedTensorProductFunctor

2a23cb5  Unicode symbol 2202 (partial) for the text display of coordinate frames

5d096f1  fstring for unicode_symbol in TensorProductFunctor and SignedTensorProductFunctor

76c2fd5  Use Unicode symbol for the Riemann sphere example

5167e6c  Use Unicode symbol for default text display of RealLine

332410b  Use unicode_otimes in TensorProductFunctor and SignedTensorProductFunctor

f5d15d2  Merge branch 'public/manifolds/unicode_art' of git://trac.sagemath.org/sage into Sage 9.4.beta4.

d87d09b  #30473: fix doctest error in DiffMap.pullback

f2ae50e  #30473: fix doctests outside sage/manifolds and sage/tensor/modules

55240bb  Merge #30473

comment:22 Changed 13 months ago by
 Dependencies set to #30473
 Status changed from needs_review to positive_review
Merged #30473 to resolve a merge conflict
comment:24 Changed 13 months ago by
 Commit changed from 55240bb9b6e4db768a2102b4065752e968a6fa18 to fff2a79b5936a2b354d80dca78612eee5301a701
Branch pushed to git repo; I updated commit sha1. New commits:
fff2a79  src/sage/sets/set.py: Fix docstring markup

comment:25 Changed 13 months ago by
 Status changed from needs_work to positive_review
comment:26 Changed 13 months ago by
 Status changed from positive_review to needs_work
 EXAMPLE:: + EXAMPLES::
comment:27 followup: ↓ 30 Changed 13 months ago by
 Status changed from needs_work to positive_review
While that is against our general practice, it is not sufficient to revert a positive review IMO. However, if Matthias wants to update it, he can go ahead and do it (and immediately reset a positive review).
comment:28 Changed 13 months ago by
 Returns the symmetric difference of ``self`` and ``X``. + Return the symmetric difference of ``self`` and ``X``.
etc.
comment:29 Changed 13 months ago by
See other tickets.
comment:30 in reply to: ↑ 27 Changed 13 months ago by
Replying to tscrim:
While that is against our general practice, it is not sufficient to revert a positive review IMO.
... in particular since this is not new code. It's not the job of a ticket to fix everything that's wrong in a given file.
comment:31 Changed 13 months ago by
comment:32 Changed 13 months ago by
I have opened a ticket devoted to those deviations of convention: #32192.
comment:33 Changed 13 months ago by
 Branch changed from u/mkoeppe/initialize_a_set_from_a_convexset_base_instance to fff2a79b5936a2b354d80dca78612eee5301a701
 Resolution set to fixed
 Status changed from positive_review to closed
Last 10 new commits:
src/sage/geometry/polyhedron/face.py: Remove unused import
Merge #31959
Merge #31990
src/sage/sets/set.py: Split out mixin classes Set_base, Set_boolean_operators, Set_add_sub_operators from Set_object; pycodestyle fixes
Polyhedron_base.*contains: Return False for noniterables  do not raise an exception
ConvexSet_base.is_finite, cardinality: New
Set_object._an_element_: Handle noniterable objects by delegating to __object.an_element
Set: Create a wrapper instance also for Elements that subclass Set_base
Set_base.union, intersection, difference, symmetric_difference: Convert to Set if necessary
ConvexSet_base.intersection: Remove abstract method; now inherits Set_base.intersection