Opened 5 years ago
Closed 5 years ago
#23990 closed defect (fixed)
Convert symbolic relations to/from SymPy
Reported by:  rws  Owned by:  

Priority:  major  Milestone:  sage8.1 
Component:  interfaces  Keywords:  
Cc:  mforets  Merged in:  
Authors:  Ralf Stephan  Reviewers:  Emmanuel Charpentier 
Report Upstream:  N/A  Work issues:  
Branch:  479e206 (Commits, GitHub, GitLab)  Commit:  479e206216ca370a9986d5e9098a912cb7656a9f 
Dependencies:  Stopgaps: 
Description (last modified by )
sage: (x>0)._sympy_() ... NotImplementedError: relation
Reversely in SymPy:
In [9]: x >= 0 Out[9]: x ≥ 0 In [10]: _._sage_()  AttributeError Traceback (most recent call last) <ipythoninput107149a2430cab> in <module>() > 1 _._sage_() AttributeError: 'GreaterThan' object has no attribute '_sage_' In [11]: Eq(x, 3) Out[11]: x = 3 In [12]: _._sage_()  AttributeError Traceback (most recent call last) <ipythoninput127149a2430cab> in <module>() > 1 _._sage_() AttributeError: 'Equality' object has no attribute '_sage_'
This functionality is needed both by #22322 and #23923. Needed is a fix to SympyConverter
in expressionconversions.pyx
The reverse patch in SymPy is added here, see the PR https://github.com/sympy/sympy/pull/13420
Change History (21)
comment:1 Changed 5 years ago by
 Branch set to u/rws/convert_symbolic_relations_to_from__sympy
comment:2 Changed 5 years ago by
 Commit set to 946295436e135f8ed5c36ea177e97c9d57f4ef98
 Description modified (diff)
 Summary changed from Convert symbolic relations to/from !SymPy to Convert symbolic relations to/from SymPy
comment:3 Changed 5 years ago by
 Commit changed from 946295436e135f8ed5c36ea177e97c9d57f4ef98 to 07f474458ffef1fc032721ae7e34720820bed1a9
Branch pushed to git repo; I updated commit sha1. New commits:
07f4744  23990: handle unequality

comment:4 Changed 5 years ago by
 Commit changed from 07f474458ffef1fc032721ae7e34720820bed1a9 to 0ed659fbff057912ecb93a97cec6b75f4cecd63a
Branch pushed to git repo; I updated commit sha1. New commits:
0ed659f  23990: do not evaluate

comment:5 Changed 5 years ago by
 Commit changed from 0ed659fbff057912ecb93a97cec6b75f4cecd63a to 5f023eafe9cfb18a38d372fcec486c2c65d15ccc
Branch pushed to git repo; I updated commit sha1. New commits:
5f023ea  23990: convert relations from SymPy to Sage, with test

comment:6 followup: ↓ 7 Changed 5 years ago by
 Cc mforets added
 Description modified (diff)
 Status changed from new to needs_review
comment:7 in reply to: ↑ 6 Changed 5 years ago by
 Reviewers set to Emmanuel Charpentier
 Status changed from needs_review to positive_review
8.1.beta7 + #23990 passes ptestlong
on Debian testing running on core i5 + 8 GB RAM with no error whatsoever.
==>positive_review
PS : You need to put your name in "Authors"
PPS : Thank you very much ! Solving inequations is a domain where Sage and Maxima are currenty weak. This is a nice step in the right direction.
comment:9 Changed 5 years ago by
P.S. Don't forget to reinstall SymPy before compiling Sage.
comment:10 followup: ↓ 15 Changed 5 years ago by
 Status changed from positive_review to needs_work
Sorry, I forgot to increase the SymPy patchlevel, and the patch does no apply anyway.
comment:11 Changed 5 years ago by
 Commit changed from 5f023eafe9cfb18a38d372fcec486c2c65d15ccc to 0f596bbdcb041c95cd846c02fea49bb38b467816
Branch pushed to git repo; I updated commit sha1. New commits:
0f596bb  23990: fix patch

comment:12 Changed 5 years ago by
 Status changed from needs_work to positive_review
comment:13 Changed 5 years ago by
 Commit changed from 0f596bbdcb041c95cd846c02fea49bb38b467816 to 479e206216ca370a9986d5e9098a912cb7656a9f
 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:
479e206  23990: sympy patchlevel bump

comment:14 Changed 5 years ago by
 Status changed from needs_review to positive_review
comment:15 in reply to: ↑ 10 Changed 5 years ago by
Replying to rws:
Sorry, I forgot to increase the SymPy patchlevel, and the patch does no apply anyway.
Ah ! I thought that the reverse conversion (which I saw on the sympy site) was reserved for another Sage patch...
comment:16 Changed 5 years ago by
 Status changed from positive_review to needs_info
sage: var("a,b") (a, b) sage: sympy.sympify(a==b)._sage_() a == b sage: sympy.sympify(a==b).sage()  AttributeError Traceback (most recent call last) <ipythoninput1425de6a4c8f0a> in <module>() > 1 sympy.sympify(a==b).sage() AttributeError: 'Equality' object has no attribute 'sage'
Is that the intended behaviour ? Or to you plan to overhaul a .sage()
method for various Sympy objects ?
It is not obvious to me why (e. g.) Mathematic objects can be converted back to Sage objects via the (exposed) .sage()
method, whereas one has to use the (unexposed) ._sage_()
methods for Sympy objects...
comment:17 followup: ↓ 18 Changed 5 years ago by
_sage_()
is just the SymPy convention, every convertible object has it. They have a lot of other underscore member functions, try grep 'def _' sympy/*/*/*
. Are Mma objects not Sage objects? Should we enforce coding conventions between projects?
Please don't forget to set back positive.
comment:18 in reply to: ↑ 17 Changed 5 years ago by
 Status changed from needs_info to positive_review
Replying to rws:
_sage_()
is just the SymPy convention, every convertible object has it. They have a lot of other underscore member functions, trygrep 'def _' sympy/*/*/*
.
Indeed. No quarell here.
Are Mma objects not Sage objects?
They are by definition Sage objects, but have no meaning for interacting with Sage "native" objects ; a conversion is needed for that.
Should we enforce coding conventions between projects?
I think that's almost what I was thinking about (syntax consistency rather than coding conventions).
Note that the nonSympy interfaces are not that consistent : compare, for example, interfaces to mathematica, giac, R, maple and fricas... Unifying them would represent a lot of work secondintention consistency is hard...
Another reason was that, unless I'm mistaken, the docs I read when I was starting to learn Sage (about 23 years ago) recommended to "end users" to use only "exposed" methods, leaving the unexposed ones to internal Sage use. I certainly had this recommendation in mind when I asked my question.
Has this recommendation changed ?
Please don't forget to set back positive.
Of course ;).
And, BTW, this was only a question, not a criticism...
comment:19 followup: ↓ 20 Changed 5 years ago by
The problem is that you seem to want a way to know how to convert a Python object from a different namespace by asking for its methods. But Sage has no control over them because Python does not allow adding methods onthefly. It may be an idea to just provide a Sage global function to_sage(...)
that knows these members and calls them without you having to know. Please open a ticket or a discussion and cite me as much as you want. This ticket has fulfilled its purpose.
comment:20 in reply to: ↑ 19 Changed 5 years ago by
Replying to rws:
...because Python does not allow adding methods onthefly...
I'm a Python noob. I didn't know about MethodType
.
comment:21 Changed 5 years ago by
 Branch changed from u/rws/convert_symbolic_relations_to_from__sympy to 479e206216ca370a9986d5e9098a912cb7656a9f
 Resolution set to fixed
 Status changed from positive_review to closed
New commits:
23990: Convert symbolic relations to SymPy