Opened 4 years ago
Closed 4 years ago
#23990 closed defect (fixed)
Convert symbolic relations to/from SymPy
Reported by: | rws | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.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) <ipython-input-10-7149a2430cab> 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) <ipython-input-12-7149a2430cab> 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 expression-conversions.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 4 years ago by
- Branch set to u/rws/convert_symbolic_relations_to_from__sympy
comment:2 Changed 4 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 4 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 4 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 4 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 follow-up: ↓ 7 Changed 4 years ago by
- Cc mforets added
- Description modified (diff)
- Status changed from new to needs_review
comment:7 in reply to: ↑ 6 Changed 4 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 4 years ago by
P.S. Don't forget to reinstall SymPy before compiling Sage.
comment:10 follow-up: ↓ 15 Changed 4 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 4 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 4 years ago by
- Status changed from needs_work to positive_review
comment:13 Changed 4 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 4 years ago by
- Status changed from needs_review to positive_review
comment:15 in reply to: ↑ 10 Changed 4 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 4 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) <ipython-input-14-25de6a4c8f0a> 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 follow-up: ↓ 18 Changed 4 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 4 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 non-Sympy interfaces are not that consistent : compare, for example, interfaces to mathematica, giac, R, maple and fricas... Unifying them would represent a lot of work second-intention consistency is hard...
Another reason was that, unless I'm mistaken, the docs I read when I was starting to learn Sage (about 2-3 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 follow-up: ↓ 20 Changed 4 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 on-the-fly. 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 4 years ago by
Replying to rws:
...because Python does not allow adding methods on-the-fly...
I'm a Python noob. I didn't know about MethodType
.
comment:21 Changed 4 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