Opened 9 months ago
Closed 7 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:  sage9.4 
Component:  manifolds  Keywords:  
Cc:  egourgoulhon, ghmjungmath  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: 
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 asdeclare_empty()
)  support 1 argument (
declare_union
is the same asdeclare_equal(other)
)
Likewise, we extend declare_intersection
and intersection
.
We also introduce
declare_disjoint(other)
 same asself.intersection(other).declare_empty()
Change History (21)
comment:1 Changed 9 months ago by
 Branch set to u/mkoeppe/manifoldsubset__union__intersection__declare__union_intersection__with_arbitrary_number_of_arguments
comment:2 Changed 9 months ago by
 Commit set to a08356971af378cabec7cd5a51be8e597ea77a67
 Status changed from new to needs_review
comment:3 Changed 9 months ago by
 Commit changed from a08356971af378cabec7cd5a51be8e597ea77a67 to da9165368b08b0115c6e141641a8282f1dcf9f5a
Branch pushed to git repo; I updated commit sha1. New commits:
da91653  ManifoldSubset._union_subset: Fixup

comment:4 Changed 8 months ago by
Doctests are missing for the methods _reduce_intersection_members
, _intersection_subset
, _reduce_union_members
and _union_subset
.
comment:5 Changed 8 months ago by
 Status changed from needs_review to needs_work
comment:6 Changed 8 months ago by
 Commit changed from da9165368b08b0115c6e141641a8282f1dcf9f5a to ee4efc9ad51725bd500380a86333eb8a69e63311
Branch pushed to git repo; I updated commit sha1. New commits:
94edd68  ManifoldSubset.declare_superset: Fix documentation

41826b4  ManifoldSubset.declare_{sub,super}set: Expand docstring

3ef5141  Merge #31763

79a625b  ManifoldSubset._reduce_intersection_members: Add examples, raise TypeError if input is empty family

8e70023  ManifoldSubset._reduce_union_members: Add examples

ee4efc9  ManifoldSubset._{union,intersection}_subset: Do cache the result here; add examples

comment:7 Changed 8 months ago by
 Status changed from needs_work to needs_review
comment:8 Changed 8 months ago by
What about the Pyflakes error? (local variable 'old' is assigned to but never used)
comment:9 Changed 8 months ago by
 Commit changed from ee4efc9ad51725bd500380a86333eb8a69e63311 to 6bbd4ae1295dbc7301571abddccb0e6c7487565d
comment:10 Changed 8 months ago by
Ready for review. The error indicated by the patchbot is unrelated to this ticket
comment:11 Changed 8 months ago by
Well, pyflakes is still not happy, despite comment:9 commit.
comment:12 Changed 8 months ago by
 Commit changed from 6bbd4ae1295dbc7301571abddccb0e6c7487565d to 2d7fda7417de199893f5b58c2e38c5096fbdb5d9
Branch pushed to git repo; I updated commit sha1. New commits:
2d7fda7  src/sage/manifolds/subset.py: Simplify code more to make pyflakes happy

comment:13 Changed 8 months ago by
Indeed... thanks for your patience
comment:14 Changed 8 months ago by
 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 8 months ago by
Thank you!
comment:16 Changed 7 months ago by
 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:
359dde1  Merge 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

0e9e5cd  Merge #31732

comment:17 Changed 7 months ago by
 Commit changed from 0e9e5cd36a4cefe81308f69429c214a4a51ab245 to a96f736b857b095ea72398bf7571936b16f7d1b8
comment:18 Changed 7 months ago by
 Commit changed from a96f736b857b095ea72398bf7571936b16f7d1b8 to 582b58d232329d0c51cb408b426fe04d30e0722a
comment:19 Changed 7 months ago by
 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 7 months ago by
 Dependencies changed from #31680, #31718, #31727, #31732, #31736, #31763 to #31680, #31718, #31727, #31732, #31736, #31763, #31880
comment:21 Changed 7 months ago by
 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
Last 10 new commits:
ManifoldSubset.intersection: Handle arbitrary intersections
intersection: Refactor
ManifoldSubset.intersection: Only pass name, latex_name to the final intersection
ManifoldSubset.intersection: Go through ManifoldSubsetFiniteFamily to make the order of operations canonical
ManifoldSubset._union_subset: Factor out from ManifoldSubset.union
ManifoldSubset.union: Handle arbitrary unions
ManifoldSubset.union: Add example and plots
ManifoldSubset.declare_union: Handle arbitrary unions
src/sage/manifolds/subset.py: import itertools
ManifoldSubset._intersection_subset, _union_subset: Use declare_subset, declare_superset; fix doctest