Opened 5 months ago

Closed 4 months ago

#32013 closed enhancement (fixed)

Initialize a Set from a ConvexSet_base instance

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.4
Component: geometry Keywords:
Cc: gh-kliem, 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:

Status badges

Description (last modified by mkoeppe)

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 non-necessarily-parent 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 3-dimensional polyhedron in ZZ^3 defined as the convex hull of 8 vertices
sage: polytopes.cube().union(polytopes.tetrahedron())                                                                                                                                                
Set-theoretic union of Set of elements of A 3-dimensional polyhedron in ZZ^3 defined as the convex hull of 8 vertices and Set of elements of A 3-dimensional polyhedron in ZZ^3 defined as the convex hull of 4 vertices

Change History (33)

comment:1 Changed 5 months ago by mkoeppe

  • Branch set to u/mkoeppe/initialize_a_set_from_a_convexset_base_instance

comment:2 Changed 5 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Cc gh-kliem tscrim added
  • Commit set to e1443896851ac805a89e8747e8c7412bbc7d9d10
  • Dependencies set to #31990
  • Description modified (diff)
  • Status changed from new to needs_review

Last 10 new commits:

f02ca28src/sage/geometry/polyhedron/face.py: Remove unused import
1744ffaMerge #31959
bd8e702Merge #31990
f351eb8src/sage/sets/set.py: Split out mix-in classes Set_base, Set_boolean_operators, Set_add_sub_operators from Set_object; pycodestyle fixes
f1db666Polyhedron_base.*contains: Return False for non-iterables - do not raise an exception
6237fbcConvexSet_base.is_finite, cardinality: New
18270ceSet_object._an_element_: Handle non-iterable objects by delegating to __object.an_element
b8f5978Set: Create a wrapper instance also for Elements that subclass Set_base
b67092cSet_base.union, intersection, difference, symmetric_difference: Convert to Set if necessary
e144389ConvexSet_base.intersection: Remove abstract method; now inherits Set_base.intersection

comment:3 Changed 5 months ago by git

  • 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 5 months ago by mkoeppe

  • Dependencies changed from #31990 to #31990, #32025

comment:5 Changed 5 months ago by mkoeppe

  • Dependencies changed from #31990, #32025 to #31990, #31877, #32025

comment:6 Changed 5 months ago by git

  • Commit changed from bc26d4f1dfef8d64ffb60db717a2db7a155771d6 to afad76f41652e85f9c1ed34504ca9e4b08e355f5

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

5b8cecaInternalRealInterval, RealSet: Add _latex_ methods
69ca854RealSet._repr_: Use unicode cup sign instead of +
dbdfc06InternalRealInterval, RealSet: Remove extra whitespace in latex, add documentation
7f56338PiecewiseFunction: Adjust doctests for changed RealSet repr
8abdc8bsrc/sage/functions/piecewise.py: Add coding header
5b0f85dMerge #31880
e768463Merge #31877
cb93c99RealSet: Inherit from Set_base, Set_boolean_operators, Set_add_sub_operators
3a6f9bdRealSet.symmetric_difference: New
afad76fsage.sets.set.Set_object: Add _sympy_ methods to subclasses

comment:7 Changed 5 months ago by git

  • Commit changed from afad76f41652e85f9c1ed34504ca9e4b08e355f5 to 34d920be955ff91868e0c144444af8fc819493fa

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

34d920bSet_base._test_as_set_object: Skip _test_pickling

comment:8 Changed 5 months ago by mkoeppe

  • Dependencies changed from #31990, #31877, #32025 to #31990, #31877

comment:9 Changed 5 months ago by git

  • Commit changed from 34d920be955ff91868e0c144444af8fc819493fa to 08c52c2535aeb4c2eadc43e22979df076a6d0760

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

08c52c2Set_base._test_as_set_object: Skip _test_pickling

comment:10 Changed 5 months ago by mkoeppe

  • Description modified (diff)

comment:11 Changed 5 months ago by git

  • Commit changed from 08c52c2535aeb4c2eadc43e22979df076a6d0760 to 200b1ef2779ae14db90d97cfc94ff5ffb2b8710b

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

200b1efMerge tag '9.4.beta4' into t/32013/initialize_a_set_from_a_convexset_base_instance

comment:12 Changed 5 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

LGTM.

comment:13 Changed 5 months ago by mkoeppe

Thanks!

comment:14 Changed 5 months ago by tscrim

  • Status changed from positive_review to needs_work

Sorry, I was a bit premature. I am getting doctest failures:

sage -t --random-seed=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 5 months ago by mkoeppe

Thanks for catching this, I'll take a look tomorrow.

comment:16 Changed 5 months ago by git

  • Commit changed from 200b1ef2779ae14db90d97cfc94ff5ffb2b8710b to 63c2cfee88ed2ded272139fd48733fdd6fab0526

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

63c2cfeConvexSet_base._test_convex_set: Do not test _test_as_set_object here

comment:17 Changed 5 months ago by mkoeppe

  • Status changed from needs_work to needs_review

Found and cherry-picked the fix for it from another ticket.

comment:18 Changed 5 months ago by mkoeppe

  • Dependencies #31990, #31877 deleted

comment:19 Changed 5 months ago by tscrim

  • Status changed from needs_review to positive_review

LGTM.

comment:20 Changed 5 months ago by mkoeppe

Thanks!

comment:21 Changed 5 months ago by git

  • 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:

da8893fUse sage.typeset.unicode_characters in TensorProductFunctor and SignedTensorProductFunctor
2a23cb5Unicode symbol 2202 (partial) for the text display of coordinate frames
5d096f1f-string for unicode_symbol in TensorProductFunctor and SignedTensorProductFunctor
76c2fd5Use Unicode symbol for the Riemann sphere example
5167e6cUse Unicode symbol for default text display of RealLine
332410bUse unicode_otimes in TensorProductFunctor and SignedTensorProductFunctor
f5d15d2Merge 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
55240bbMerge #30473

comment:22 Changed 5 months ago by mkoeppe

  • Dependencies set to #30473
  • Status changed from needs_review to positive_review

Merged #30473 to resolve a merge conflict

comment:23 Changed 5 months ago by vbraun

  • Status changed from positive_review to needs_work

PDF docs fail

comment:24 Changed 5 months ago by git

  • Commit changed from 55240bb9b6e4db768a2102b4065752e968a6fa18 to fff2a79b5936a2b354d80dca78612eee5301a701

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

fff2a79src/sage/sets/set.py: Fix docstring markup

comment:25 Changed 5 months ago by mkoeppe

  • Status changed from needs_work to positive_review

comment:26 Changed 5 months ago by gh-mjungmath

  • Status changed from positive_review to needs_work
-        EXAMPLE::
+        EXAMPLES::

See https://wiki.sagemath.org/plugins#blocks.

Version 0, edited 5 months ago by gh-mjungmath (next)

comment:27 follow-up: Changed 5 months ago by tscrim

  • 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 5 months ago by gh-mjungmath

-        Returns the symmetric difference of ``self`` and ``X``.
+        Return the symmetric difference of ``self`` and ``X``.

etc.

See https://wiki.sagemath.org/plugins#blocks.

comment:29 Changed 5 months ago by tscrim

See other tickets.

comment:30 in reply to: ↑ 27 Changed 5 months ago by mkoeppe

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.

Last edited 5 months ago by mkoeppe (previous) (diff)

comment:31 Changed 5 months ago by gh-mjungmath

Last edited 5 months ago by gh-mjungmath (previous) (diff)

comment:32 Changed 5 months ago by gh-mjungmath

I have opened a ticket devoted to those deviations of convention: #32192.

comment:33 Changed 4 months ago by vbraun

  • 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
Note: See TracTickets for help on using tickets.