#32015 closed enhancement (fixed)

_sympy_ methods for Set_object_binary subclasses

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-9.4
Component: interfaces Keywords:
Cc: Travis Scrimshaw, Karl-Dieter Crisman, Eric Gourgoulhon 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:

Status badges

Description (last modified by Matthias Köppe)

Formal unions, intersections, ...

Change History (26)

comment:1 Changed 18 months ago by Matthias Köppe

Description: modified (diff)

comment:2 Changed 18 months ago by Matthias Köppe

Branch: u/mkoeppe/_sympy__methods_for_set_object_binary_subclasses

comment:3 Changed 18 months ago by Matthias Köppe

Authors: Matthias Koeppe
Cc: Karl-Dieter Crisman added
Commit: 3dd50e485860e39ce19973eba58fc9f0ed43ba0b
Status: newneeds_review

Last 10 new commits:

6e5cac6sage.interfaces.sympy_wrapper, Sets.ParentMethods._sympy_: New
3cac256sage.interfaces.sympy_wrapper: Add doctests
eef604eSageSet: Finish docstrings; handle symbolic _contains
2baae58Sets.ParentMethods._sympy_: Call sympy_init
153b3e5Merge #31938
c06c965sage.interfaces.sympy_wrapper.SageSet: Add another doctest
f535127Merge #31938
32cdd5cMerge #31877
789dc05Merge #31931
3dd50e4sage.sets.set.Set_object: Add _sympy_ methods to subclasses

comment:4 Changed 17 months ago by Matthias Köppe

Cc: Eric Gourgoulhon added
Dependencies: #32013, #31931, #31938#32013

comment:5 Changed 17 months ago by git

Commit: 3dd50e485860e39ce19973eba58fc9f0ed43ba0b7f35aee31a94d4c22ce604b6e5ba7ec34c260720

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

200b1efMerge tag '9.4.beta4' into t/32013/initialize_a_set_from_a_convexset_base_instance
7f35aeeMerge #32013

comment:6 Changed 17 months ago by Travis Scrimshaw

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 17 months ago by Matthias Köppe

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 17 months ago by Travis Scrimshaw

Reviewers: 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 17 months ago by Matthias Köppe

Status: needs_reviewpositive_review

Thanks! I'm keeping it as is.

comment:10 Changed 17 months ago by Volker Braun

Status: positive_reviewneeds_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 17 months ago by git

Commit: 7f35aee31a94d4c22ce604b6e5ba7ec34c260720daeb91e7313d4240002e573078fb5547cc3a8a78

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

daeb91esrc/sage/sets/set.py: Fix docstring markup

comment:12 Changed 17 months ago by Matthias Köppe

Status: needs_workpositive_review

comment:13 Changed 17 months ago by Volker Braun

Status: positive_reviewneeds_work

Tests fail

comment:14 Changed 17 months ago by git

Commit: daeb91e7313d4240002e573078fb5547cc3a8a7826824697667f2eec0977b532f442e8e2ed4a8058

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

2682469src/sage/interfaces/sympy_wrapper.py: Use Family, not Set, in doctests to make sure that the SageSet wrapper is actually used

comment:15 Changed 17 months ago by git

Commit: 26824697667f2eec0977b532f442e8e2ed4a8058753babb46c7bbe70faa1e0da4d7887b4fa5867e0

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

753babbSet_object_enumerated._sympy_: Translate empty sets to EmptySet

comment:16 Changed 17 months ago by Matthias Köppe

Status: needs_workneeds_review

comment:17 Changed 17 months ago by Travis Scrimshaw

Status: needs_reviewpositive_review

LGTM.

comment:18 Changed 17 months ago by Michael Jung

Status: positive_reviewneeds_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 17 months ago by git

Commit: 753babb46c7bbe70faa1e0da4d7887b4fa5867e0451f5cf5fb60611db32bf5fbcdecfec230c087f6

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

451f5cfSets.ParentMethods: Update doctest

comment:20 Changed 17 months ago by Matthias Köppe

Status: needs_workneeds_review

comment:21 Changed 17 months ago by Travis Scrimshaw

Status: needs_reviewpositive_review

Thanks for catching that.

comment:22 Changed 17 months ago by Michael Jung

Reviewers: Travis ScrimshawTravis 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 17 months ago by Matthias Köppe

Thanks!

comment:24 Changed 17 months ago by git

Commit: 451f5cf5fb60611db32bf5fbcdecfec230c087f6689986b2a5f5d9b839af7c0b1dd13910d20cb928
Status: positive_reviewneeds_review

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

63c2cfeConvexSet_base._test_convex_set: Do not test _test_as_set_object here
55240bbMerge #30473
fff2a79src/sage/sets/set.py: Fix docstring markup
70a2e7dMerge #32013
689986bMerge tag '9.4.beta5' into t/32015/_sympy__methods_for_set_object_binary_subclasses

comment:25 Changed 17 months ago by Matthias Köppe

Status: needs_reviewpositive_review

comment:26 Changed 17 months ago by Volker Braun

Branch: u/mkoeppe/_sympy__methods_for_set_object_binary_subclasses689986b2a5f5d9b839af7c0b1dd13910d20cb928
Resolution: fixed
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.