Opened 13 months ago
Closed 11 months ago
#32015 closed enhancement (fixed)
_sympy_ methods for Set_object_binary subclasses
Reported by: | mkoeppe | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.4 |
Component: | interfaces | Keywords: | |
Cc: | tscrim, kcrisman, egourgoulhon | Merged in: | |
Authors: | Matthias Koeppe | Reviewers: | Travis Scrimshaw, Michael Jung |
Report Upstream: | N/A | Work issues: | |
Branch: | 689986b (Commits, GitHub, GitLab) | Commit: | 689986b2a5f5d9b839af7c0b1dd13910d20cb928 |
Dependencies: | #32013 | Stopgaps: |
Description (last modified by )
Formal unions, intersections, ...
Change History (26)
comment:1 Changed 13 months ago by
- Description modified (diff)
comment:2 Changed 13 months ago by
- Branch set to u/mkoeppe/_sympy__methods_for_set_object_binary_subclasses
comment:3 Changed 13 months ago by
- Cc kcrisman added
- Commit set to 3dd50e485860e39ce19973eba58fc9f0ed43ba0b
- Status changed from new to needs_review
comment:4 Changed 12 months ago by
- Cc egourgoulhon added
- Dependencies changed from #32013, #31931, #31938 to #32013
comment:5 Changed 12 months ago by
- Commit changed from 3dd50e485860e39ce19973eba58fc9f0ed43ba0b to 7f35aee31a94d4c22ce604b6e5ba7ec34c260720
comment:6 Changed 12 months ago by
I am not sure about caching the output of _sympy_
? It creates a clone of the set and ties it to the Sage object. Can you give some reasoning for this?
comment:7 Changed 12 months ago by
If one makes one computation with the sympy version of a set, it's likely that another computation will be made -- so saving the result seemed a good idea.
comment:8 Changed 12 months ago by
- Reviewers set to Travis Scrimshaw
I would expect someone to store said Sympy version if they were going to use it again. Granted, I expect this to not be a problem either way. My instinct is to err on the side of not caching, but I don't have a strong opinion on this. So you either change it or not; once decided, you can set a positive review.
comment:9 Changed 12 months ago by
- Status changed from needs_review to positive_review
Thanks! I'm keeping it as is.
comment:10 Changed 12 months ago by
- Status changed from positive_review to needs_work
[docpdf] [docpdf] Overfull \hbox (22.51231p [docpdf] ! Missing { inserted. [docpdf] <to be read again> [docpdf] _ [docpdf] l.1488 ...n{}in class providing the operators \(__ [docpdf] add__\) and \(__sub__\). [docpdf] ? [docpdf] [809 [docpdf] ! Emergency stop. [docpdf] <to be read again> [docpdf] _ [docpdf] l.1488 ...n{}in class providing the operators \(__ [docpdf] add__\) and \(__sub__\). [docpdf] ! ==> Fatal error occurred, no output PDF file produced!
comment:11 Changed 12 months ago by
- Commit changed from 7f35aee31a94d4c22ce604b6e5ba7ec34c260720 to daeb91e7313d4240002e573078fb5547cc3a8a78
Branch pushed to git repo; I updated commit sha1. New commits:
daeb91e | src/sage/sets/set.py: Fix docstring markup
|
comment:12 Changed 12 months ago by
- Status changed from needs_work to positive_review
comment:14 Changed 12 months ago by
- Commit changed from daeb91e7313d4240002e573078fb5547cc3a8a78 to 26824697667f2eec0977b532f442e8e2ed4a8058
Branch pushed to git repo; I updated commit sha1. New commits:
2682469 | src/sage/interfaces/sympy_wrapper.py: Use Family, not Set, in doctests to make sure that the SageSet wrapper is actually used
|
comment:15 Changed 12 months ago by
- Commit changed from 26824697667f2eec0977b532f442e8e2ed4a8058 to 753babb46c7bbe70faa1e0da4d7887b4fa5867e0
Branch pushed to git repo; I updated commit sha1. New commits:
753babb | Set_object_enumerated._sympy_: Translate empty sets to EmptySet
|
comment:16 Changed 12 months ago by
- Status changed from needs_work to needs_review
comment:18 Changed 12 months ago by
- Status changed from positive_review to needs_work
File "src/sage/categories/sets_cat.py", line 1720, in sage.categories.sets_cat.Sets.ParentMethods._sympy_ Failed example: sF = F._sympy_(); sF Expected: SageSet({1, 2}) Got: Set(1, 2)
and
File "src/sage/categories/sets_cat.py", line 1722, in sage.categories.sets_cat.Sets.ParentMethods._sympy_ Failed example: sF._sage_() is F Exception raised: Traceback (most recent call last): File "/data/pell/lipnik/patchbot/local/lib/python3.9/site-packages/sage/doctest/forker.py", line 718, in _run self.compile_and_execute(example, compiler, test.globs) File "/data/pell/lipnik/patchbot/local/lib/python3.9/site-packages/sage/doctest/forker.py", line 1137, in compile_and_execute exec(compiled, globs) File "<doctest sage.categories.sets_cat.Sets.ParentMethods._sympy_[12]>", line 1, in <module> sF._sage_() is F AttributeError: 'Set' object has no attribute '_sage_'
comment:19 Changed 12 months ago by
- Commit changed from 753babb46c7bbe70faa1e0da4d7887b4fa5867e0 to 451f5cf5fb60611db32bf5fbcdecfec230c087f6
Branch pushed to git repo; I updated commit sha1. New commits:
451f5cf | Sets.ParentMethods: Update doctest
|
comment:20 Changed 12 months ago by
- Status changed from needs_work to needs_review
comment:21 Changed 12 months ago by
- Status changed from needs_review to positive_review
Thanks for catching that.
comment:22 Changed 12 months ago by
- Reviewers changed from Travis Scrimshaw to Travis Scrimshaw, Michael Jung
sage -t --random-seed=0 src/sage/categories/sets_cat.py [478 tests, 0.52 s] ---------------------------------------------------------------------- All tests passed! ----------------------------------------------------------------------
comment:23 Changed 12 months ago by
Thanks!
comment:24 Changed 12 months ago by
- Commit changed from 451f5cf5fb60611db32bf5fbcdecfec230c087f6 to 689986b2a5f5d9b839af7c0b1dd13910d20cb928
- 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:
63c2cfe | ConvexSet_base._test_convex_set: Do not test _test_as_set_object here
|
55240bb | Merge #30473
|
fff2a79 | src/sage/sets/set.py: Fix docstring markup
|
70a2e7d | Merge #32013
|
689986b | Merge tag '9.4.beta5' into t/32015/_sympy__methods_for_set_object_binary_subclasses
|
comment:25 Changed 12 months ago by
- Status changed from needs_review to positive_review
comment:26 Changed 11 months ago by
- Branch changed from u/mkoeppe/_sympy__methods_for_set_object_binary_subclasses to 689986b2a5f5d9b839af7c0b1dd13910d20cb928
- Resolution set to fixed
- Status changed from positive_review to closed
Last 10 new commits:
sage.interfaces.sympy_wrapper, Sets.ParentMethods._sympy_: New
sage.interfaces.sympy_wrapper: Add doctests
SageSet: Finish docstrings; handle symbolic _contains
Sets.ParentMethods._sympy_: Call sympy_init
Merge #31938
sage.interfaces.sympy_wrapper.SageSet: Add another doctest
Merge #31938
Merge #31877
Merge #31931
sage.sets.set.Set_object: Add _sympy_ methods to subclasses