#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) Commit: 479e206216ca370a9986d5e9098a912cb7656a9f
Dependencies: Stopgaps:

Description (last modified by rws)

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 18 months ago by rws

  • Branch set to u/rws/convert_symbolic_relations_to_from__sympy

comment:2 Changed 18 months ago by rws

  • Commit set to 946295436e135f8ed5c36ea177e97c9d57f4ef98
  • Description modified (diff)
  • Summary changed from Convert symbolic relations to/from !SymPy to Convert symbolic relations to/from SymPy

New commits:

946295423990: Convert symbolic relations to SymPy

comment:3 Changed 18 months ago by git

  • Commit changed from 946295436e135f8ed5c36ea177e97c9d57f4ef98 to 07f474458ffef1fc032721ae7e34720820bed1a9

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

07f474423990: handle unequality

comment:4 Changed 18 months ago by git

  • Commit changed from 07f474458ffef1fc032721ae7e34720820bed1a9 to 0ed659fbff057912ecb93a97cec6b75f4cecd63a

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

0ed659f23990: do not evaluate

comment:5 Changed 18 months ago by git

  • Commit changed from 0ed659fbff057912ecb93a97cec6b75f4cecd63a to 5f023eafe9cfb18a38d372fcec486c2c65d15ccc

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

5f023ea23990: convert relations from SymPy to Sage, with test

comment:6 follow-up: Changed 18 months ago by rws

  • Cc mforets added
  • Description modified (diff)
  • Status changed from new to needs_review

comment:7 in reply to: ↑ 6 Changed 18 months ago by charpent

  • 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:8 Changed 18 months ago by rws

  • Authors set to Ralf Stephan

Thanks!

comment:9 Changed 18 months ago by rws

P.S. Don't forget to reinstall SymPy before compiling Sage.

comment:10 follow-up: Changed 18 months ago by rws

  • 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 18 months ago by git

  • Commit changed from 5f023eafe9cfb18a38d372fcec486c2c65d15ccc to 0f596bbdcb041c95cd846c02fea49bb38b467816

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

0f596bb23990: fix patch

comment:12 Changed 18 months ago by rws

  • Status changed from needs_work to positive_review

comment:13 Changed 18 months ago by git

  • 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:

479e20623990: sympy patchlevel bump

comment:14 Changed 18 months ago by rws

  • Status changed from needs_review to positive_review

comment:15 in reply to: ↑ 10 Changed 18 months ago by charpent

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 18 months ago by charpent

  • 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: Changed 18 months ago by rws

_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 18 months ago by charpent

  • 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, try grep '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: Changed 18 months ago by rws

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 18 months ago by rws

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

  • Branch changed from u/rws/convert_symbolic_relations_to_from__sympy to 479e206216ca370a9986d5e9098a912cb7656a9f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.