Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
mkoeppe opened this issue May 2, 2021 · 30 comments

Comments

@mkoeppe
Copy link
Member

mkoeppe commented May 2, 2021

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()

Depends on #31680
Depends on #31718
Depends on #31727
Depends on #31732
Depends on #31736
Depends on #31763
Depends on #31880

CC: @egourgoulhon @mjungmath

Component: manifolds

Author: Matthias Koeppe

Branch/Commit: 582b58d

Reviewer: Eric Gourgoulhon

Issue created by migration from https://trac.sagemath.org/ticket/31764

@mkoeppe mkoeppe added this to the sage-9.4 milestone May 2, 2021
@mkoeppe
Copy link
Member Author

mkoeppe commented May 2, 2021

@mkoeppe
Copy link
Member Author

mkoeppe commented May 2, 2021

Commit: a083569

@mkoeppe
Copy link
Member Author

mkoeppe commented May 2, 2021

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

@mkoeppe
Copy link
Member Author

mkoeppe commented May 2, 2021

Author: Matthias Koeppe

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2021

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

da91653ManifoldSubset._union_subset: Fixup

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 8, 2021

Changed commit from a083569 to da91653

@egourgoulhon
Copy link
Member

comment:4

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 11, 2021

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 11, 2021

Changed commit from da91653 to ee4efc9

@egourgoulhon
Copy link
Member

comment:8

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 12, 2021

Changed commit from ee4efc9 to 6bbd4ae

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 12, 2021

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

@mkoeppe
Copy link
Member Author

mkoeppe commented May 16, 2021

comment:10

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

@egourgoulhon
Copy link
Member

comment:11

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 17, 2021

Changed commit from 6bbd4ae to 2d7fda7

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented May 17, 2021

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

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

@mkoeppe
Copy link
Member Author

mkoeppe commented May 17, 2021

comment:13

Indeed... thanks for your patience

@egourgoulhon
Copy link
Member

comment:14

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

@egourgoulhon
Copy link
Member

Reviewer: Eric Gourgoulhon

@mkoeppe
Copy link
Member Author

mkoeppe commented May 19, 2021

comment:15

Thank you!

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 19, 2021

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 19, 2021

Changed commit from 2d7fda7 to 0e9e5cd

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 19, 2021

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

fd4506aMerge #31732
a96f736Merge #31736

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 19, 2021

Changed commit from 0e9e5cd to a96f736

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 19, 2021

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

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jun 19, 2021

Changed commit from a96f736 to 582b58d

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 19, 2021

comment:19

Trivial merge with updated tickets, includes latest beta.

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 19, 2021

Changed dependencies from #31680, #31718, #31677, #31727, #31732, #31736, #31763 to #31680, #31718, #31727, #31732, #31736, #31763

@mkoeppe
Copy link
Member Author

mkoeppe commented Jun 20, 2021

Changed dependencies from #31680, #31718, #31727, #31732, #31736, #31763 to #31680, #31718, #31727, #31732, #31736, #31763, #31880

@vbraun
Copy link
Member

vbraun commented Jun 21, 2021

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants