Opened 6 months ago

Closed 4 months ago

#31764 closed enhancement (fixed)

ManifoldSubset: union, intersection, declare_{union,intersection} with arbitrary number of arguments

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.4
Component: manifolds Keywords:
Cc: egourgoulhon, gh-mjungmath Merged in:
Authors: Matthias Koeppe Reviewers: Eric Gourgoulhon
Report Upstream: N/A Work issues:
Branch: 582b58d (Commits, GitHub, GitLab) Commit: 582b58d232329d0c51cb408b426fe04d30e0722a
Dependencies: #31680, #31718, #31727, #31732, #31736, #31763, #31880 Stopgaps:

Status badges

Description

In this ticket, we extend union and declare_union as follows:

  • support more than two arguments
  • optional argument disjoint
  • support 0 arguments (declare_union is the same as declare_empty())
  • support 1 argument (declare_union is the same as declare_equal(other))

Likewise, we extend declare_intersection and intersection.

We also introduce

  • declare_disjoint(other) - same as self.intersection(other).declare_empty()

Change History (21)

comment:1 Changed 6 months ago by mkoeppe

  • Branch set to u/mkoeppe/manifoldsubset__union__intersection__declare__union_intersection__with_arbitrary_number_of_arguments

comment:2 Changed 6 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Commit set to a08356971af378cabec7cd5a51be8e597ea77a67
  • Status changed from new to needs_review

Last 10 new commits:

c776366ManifoldSubset.intersection: Handle arbitrary intersections
fb6e66aintersection: Refactor
b1bb1aaManifoldSubset.intersection: Only pass name, latex_name to the final intersection
0032bf9ManifoldSubset.intersection: Go through ManifoldSubsetFiniteFamily to make the order of operations canonical
282010bManifoldSubset._union_subset: Factor out from ManifoldSubset.union
6772570ManifoldSubset.union: Handle arbitrary unions
edfea33ManifoldSubset.union: Add example and plots
fdd0abaManifoldSubset.declare_union: Handle arbitrary unions
c2f484csrc/sage/manifolds/subset.py: import itertools
a083569ManifoldSubset._intersection_subset, _union_subset: Use declare_subset, declare_superset; fix doctest

comment:3 Changed 5 months ago by git

  • Commit changed from a08356971af378cabec7cd5a51be8e597ea77a67 to da9165368b08b0115c6e141641a8282f1dcf9f5a

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

da91653ManifoldSubset._union_subset: Fixup

comment:4 Changed 5 months ago by egourgoulhon

Doctests are missing for the methods _reduce_intersection_members, _intersection_subset, _reduce_union_members and _union_subset.

comment:5 Changed 5 months ago by mkoeppe

  • Status changed from needs_review to needs_work

comment:6 Changed 5 months ago by git

  • Commit changed from da9165368b08b0115c6e141641a8282f1dcf9f5a to ee4efc9ad51725bd500380a86333eb8a69e63311

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

94edd68ManifoldSubset.declare_superset: Fix documentation
41826b4ManifoldSubset.declare_{sub,super}set: Expand docstring
3ef5141Merge #31763
79a625bManifoldSubset._reduce_intersection_members: Add examples, raise TypeError if input is empty family
8e70023ManifoldSubset._reduce_union_members: Add examples
ee4efc9ManifoldSubset._{union,intersection}_subset: Do cache the result here; add examples

comment:7 Changed 5 months ago by mkoeppe

  • Status changed from needs_work to needs_review

comment:8 Changed 5 months ago by egourgoulhon

What about the Pyflakes error? (local variable 'old' is assigned to but never used)

comment:9 Changed 5 months ago by git

  • Commit changed from ee4efc9ad51725bd500380a86333eb8a69e63311 to 6bbd4ae1295dbc7301571abddccb0e6c7487565d

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

21739desrc/sage/manifolds/subset.py: Simplify code, make pyflakes happy
6bbd4aesrc/sage/manifolds/subset.py: Add coding header

comment:10 Changed 5 months ago by mkoeppe

Ready for review. The error indicated by the patchbot is unrelated to this ticket

comment:11 Changed 5 months ago by egourgoulhon

Well, pyflakes is still not happy, despite comment:9 commit.

comment:12 Changed 5 months ago by git

  • Commit changed from 6bbd4ae1295dbc7301571abddccb0e6c7487565d to 2d7fda7417de199893f5b58c2e38c5096fbdb5d9

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

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

comment:13 Changed 5 months ago by mkoeppe

Indeed... thanks for your patience

comment:14 Changed 5 months ago by egourgoulhon

  • Reviewers set to Eric Gourgoulhon
  • Status changed from needs_review to positive_review

Sorry for the delay. LGTM. Thanks for this improvement!

comment:15 Changed 5 months ago by mkoeppe

Thank you!

comment:16 Changed 4 months ago by git

  • Commit changed from 2d7fda7417de199893f5b58c2e38c5096fbdb5d9 to 0e9e5cd36a4cefe81308f69429c214a4a51ab245
  • 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:

359dde1Merge branch 't/31727/manifoldsubset__add_methods_subset_family__superset_family__equal_subset_family__deprecate_method_list_of_subsets' into t/31732/manifoldsubset__new_methods_declare_empty__declare_nonempty__is_empty__has_defined_points__open_cover_family
0e9e5cdMerge #31732

comment:17 Changed 4 months ago by git

  • Commit changed from 0e9e5cd36a4cefe81308f69429c214a4a51ab245 to a96f736b857b095ea72398bf7571936b16f7d1b8

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

fd4506aMerge #31732
a96f736Merge #31736

comment:18 Changed 4 months ago by git

  • Commit changed from a96f736b857b095ea72398bf7571936b16f7d1b8 to 582b58d232329d0c51cb408b426fe04d30e0722a

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

7fed9efMerge #31736
c4acd09Merge tag '9.4.beta2' into t/31763/manifoldsubset__new_methods_declare_subset__declare_superset
582b58dMerge #31763

comment:19 Changed 4 months ago by mkoeppe

  • Dependencies changed from #31680, #31718, #31677, #31727, #31732, #31736, #31763 to #31680, #31718, #31727, #31732, #31736, #31763
  • Status changed from needs_review to positive_review

Trivial merge with updated tickets, includes latest beta.

comment:20 Changed 4 months ago by mkoeppe

  • Dependencies changed from #31680, #31718, #31727, #31732, #31736, #31763 to #31680, #31718, #31727, #31732, #31736, #31763, #31880

comment:21 Changed 4 months ago by vbraun

  • Branch changed from u/mkoeppe/manifoldsubset__union__intersection__declare__union_intersection__with_arbitrary_number_of_arguments to 582b58d232329d0c51cb408b426fe04d30e0722a
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.