Opened 7 months ago

Closed 6 months ago

#31798 closed enhancement (fixed)

ManifoldSubset.difference, complement

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: 2113f78 (Commits, GitHub, GitLab) Commit: 2113f788564abcfe08176998b4d2132b11b5aede
Dependencies: #31764 Stopgaps:

Status badges

Description

We implement ManifoldSubset.difference by making a new subset and declaring a disjoint union (#31764)

Change History (16)

comment:1 Changed 7 months ago by mkoeppe

  • Branch set to u/mkoeppe/manifoldsubset_difference__complement

comment:2 Changed 7 months ago by mkoeppe

  • Commit set to 3786cee241f6097efc8a695dc3f9b6913584f4b7
  • Status changed from new to needs_review

Last 10 new commits:

​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
​da91653ManifoldSubset._union_subset: Fixup
​3e6a9cfMerge #31764
​3786ceeManifoldSubset.difference, complement: New

comment:3 Changed 7 months ago by gh-mjungmath

  • Status changed from needs_review to needs_work

Patchbot seems not to like it.

comment:4 Changed 7 months ago by git

  • Commit changed from 3786cee241f6097efc8a695dc3f9b6913584f4b7 to 6370e88a31ff2eba67b753ec1bacb424dc86aa5d

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

​6370e88ManifoldSubset.difference, complement: New

comment:5 follow-up: Changed 7 months ago by mkoeppe

  • Status changed from needs_work to needs_review

Let's see what the patchbot thinks now

comment:6 in reply to: ↑ 5 Changed 7 months ago by egourgoulhon

Replying to mkoeppe:

Let's see what the patchbot thinks now

He's still not happy, but this seems due to the lack of coverage in the dependency #31764.

comment:7 Changed 7 months ago by egourgoulhon

Btw, with Python 3, shouldn't the non_ascii pluggin be removed from the pathbot? It generates a false alarm here.

Version 0, edited 7 months ago by egourgoulhon (next)

comment:8 Changed 7 months ago by git

  • Commit changed from 6370e88a31ff2eba67b753ec1bacb424dc86aa5d to e7f7a7d4445ebdcb01b1dbfc42205c71e7b276e9

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
​e7f7a7dMerge #31764

comment:9 Changed 7 months ago by git

  • Commit changed from e7f7a7d4445ebdcb01b1dbfc42205c71e7b276e9 to 8d3ad854f87a640e7e0713deffd3dd73c42991e1

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
​2d7fda7src/sage/manifolds/subset.py: Simplify code more to make pyflakes happy
​8d3ad85Merge #31764

comment:10 Changed 7 months ago by egourgoulhon

I don't understand the pyflakes errors reported by the patchbot. Do you?

comment:11 follow-up: Changed 7 months ago by mkoeppe

The patchbot report at the very top refers to the commit before I merged in the latest version of #31764.

comment:12 in reply to: ↑ 11 Changed 7 months ago by egourgoulhon

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

Replying to mkoeppe:

The patchbot report at the very top refers to the commit before I merged in the latest version of #31764.

That's strange. Given the dates, this patchbot should have run on the latest commit. Anyway, everything is fine. Thanks for these new functionalities!

comment:13 Changed 7 months ago by mkoeppe

Thanks for the review!

comment:14 Changed 6 months ago by git

  • Commit changed from 8d3ad854f87a640e7e0713deffd3dd73c42991e1 to 2113f788564abcfe08176998b4d2132b11b5aede
  • 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
​fd4506aMerge #31732
​a96f736Merge #31736
​7fed9efMerge #31736
​c4acd09Merge tag '9.4.beta2' into t/31763/manifoldsubset__new_methods_declare_subset__declare_superset
​582b58dMerge #31763
​2113f78Merge #31764

comment:15 Changed 6 months ago by mkoeppe

  • Status changed from needs_review to positive_review

Trivial merge with update dependency

comment:16 Changed 6 months ago by vbraun

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