Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The value of str(expr) depends on a global setting #5487

Closed
rlamy opened this issue May 18, 2011 · 16 comments
Closed

The value of str(expr) depends on a global setting #5487

rlamy opened this issue May 18, 2011 · 16 comments

Comments

@rlamy
Copy link
Member

rlamy commented May 18, 2011

With 'bin/isympy':
In [1]: str(x+y+x*y+1)
Out[1]: 1 + x + y + x*y

With 'bin/isympy -o lex':
In [1]: str(x+y+x*y+1)
Out[1]: x*y + x + y + 1

Having the result of such an elementary function vary according to mysterious circumstances is bad.

Original issue for #5487: http://code.google.com/p/sympy/issues/detail?id=2388
Original author: https://code.google.com/u/101272611947379421629/
Original owner: https://code.google.com/u/101272611947379421629/

@asmeurer
Copy link
Member

But that's how the printer works in SymPy.  Having it *not* respect the -o option would be even more confusing, imo.  

How would you do it instead?

**Status:** NeedsDecision  

Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c1
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

Ronan, do you have a better way to do it?  We can't block the release on something that doesn't even have a solution.

**Labels:** -Milestone-Release0.7.0 Milestone-Release0.7.1  

Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c2
Original author: https://code.google.com/u/asmeurer@gmail.com/

@rlamy
Copy link
Member Author

rlamy commented May 19, 2011

The -o option should change sys.displayhook, but leave str() alone. As I said, this is a basic function and people don't expect its output to be affected by the phase of the moon.

**Labels:** -Milestone-Release0.7.1 Milestone-Release0.7.0  

Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c3
Original author: https://code.google.com/u/101272611947379421629/

@asmeurer
Copy link
Member

I suppose that's reasonable.  And we do have sstr(), which could use the global option.  Any objections?

We should make str() use lex, though (as per issue 4590 ).

Referenced issues: #4590
Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c4
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

**Status:** Accepted  
**Cc:** matt...@gmail.com  
**Blockedon:** 1491  

Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c5
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

OK, so two questions:

- Should repr() be the same?

- Should __repr__ call srepr()?  Right now, it calls sstr() (the same as __str__).

I'll submit a patch soon based on the assumption that the answer to both of those questions is "yes".

Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c6
Original author: https://code.google.com/u/asmeurer@gmail.com/

@asmeurer
Copy link
Member

**Blockedon:** 5516  

Referenced issues: #5516
Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c7
Original author: https://code.google.com/u/asmeurer@gmail.com/

@mattpap
Copy link
Member

mattpap commented May 23, 2011

It calls srepr() because of a bug in Python. If you change Basic.__repr__ to call sstr() then you will get:

In [1]: str([1/x, 1/y])
Out[1]: [Pow(Symbol('x'), Integer(-1)), Pow(Symbol('y'), Integer(-1))]

Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c8
Original author: https://code.google.com/u/101069955704897915480/

@mattpap
Copy link
Member

mattpap commented May 23, 2011

Of course please swap srepr() with sstr(). It should read "It calls sstr() because of a bug in Python. If you change Basic.__repr__ to call srepr() (...)".

Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c9
Original author: https://code.google.com/u/101069955704897915480/

@asmeurer
Copy link
Member

I see.  Well, that's incredibly stupid.  Do the Python guys know about this?

Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c10
Original author: https://code.google.com/u/asmeurer@gmail.com/

@mattpap
Copy link
Member

mattpap commented May 23, 2011

Yes, it is. Ondrej asked about this on python-dev: http://mail.python.org/pipermail/python-dev/2008-July/081562.html There is also a rejected PEP for Py3k: http://www.python.org/dev/peps/pep-3140/

Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c11
Original author: https://code.google.com/u/101069955704897915480/

@asmeurer
Copy link
Member

The patch at https://github.com/sympy/sympy/pull/350 fixes this, btw.

**Labels:** NeedsReview asmeurer  

Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c12
Original author: https://code.google.com/u/asmeurer@gmail.com/

@rlamy
Copy link
Member Author

rlamy commented May 27, 2011

It doesn't seem to actually be a bug in Python - I searched through the tracker. From the discussions linked, it seems to be considered a feature. We have to either put up with it or use another language. OTOH, the fact that __repr__ = __str__ for sympy objects is clearly a bug, because it doesn't conform to the definition of builtin repr().

Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c13
Original author: https://code.google.com/u/101272611947379421629/

@asmeurer
Copy link
Member

Well, the Python guys don't consider it to be a "bug" per se, but I definitely do.  

If we make __repr__ == srepr, then [10] below would give the same things as [9]:

In [9]: srepr([x**2 + x + 1, exp(sin(1/x + 2))])
Out[9]: [Add(Integer(1), Symbol('x'), Pow(Symbol('x'), Integer(2))), Function('exp')(Function('sin')(Add(Integer(2), Pow(Symbol('x'), Integer(-1)))))]

In [10]: print ([x**2 + x + 1, exp(sin(1/x + 2))])
[x**2 + x + 1, exp(sin(2 + 1/x))]

Based on that python-dev thread, this has probably been discussed before, either here on the issue tracker or on the mailing list.  We should try to find that.

Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c14
Original author: https://code.google.com/u/asmeurer@gmail.com/

@vks
Copy link
Contributor

vks commented May 27, 2011

I remember the discussion. Basically they said that str(list(...)) will always use repr() on the content of the list. This is why we decided to ignore the separation of __str__ and __repr__ in favor of our own functions.

We could think about subclassing list and reimplementing __str__ on the other hand, to allow a more conformal __repr__ implementation.

Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c15
Original author: https://code.google.com/u/Vinzent.Steinberg@gmail.com/

@rlamy
Copy link
Member Author

rlamy commented Jun 1, 2011

It's in.

**Status:** Fixed  

Original comment: http://code.google.com/p/sympy/issues/detail?id=2388#c16
Original author: https://code.google.com/u/101272611947379421629/

@rlamy rlamy added this to the Release0.7.0 milestone Mar 7, 2014
@rlamy rlamy self-assigned this Mar 7, 2014
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants