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

Sets printing issues #10672

Closed
asmeurer opened this issue Feb 23, 2016 · 23 comments · Fixed by #15635
Closed

Sets printing issues #10672

asmeurer opened this issue Feb 23, 2016 · 23 comments · Fixed by #15635
Labels
Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. printing sets

Comments

@asmeurer
Copy link
Member

Some issues with str and srepr printing in sets.

Some notes:

  • The str printer should always generate valid Python, which recreates the expression (but may require some variables to be defined).
  • The srepr printer should generate an expression that recreates the expression exactly, using only the names from from sympy import * (or other relevant imports for other submodules, but that isn't relevant for the sets).
  • Fancy printing should be relegated to the pretty printers (pprint and latex).

Here are the issues I found:

  1. str(Interval)

    In [9]: str(Interval(0, 1, False))
    Out[9]: '[0, 1]'
    
    In [10]: str(Interval(0, 1, True))
    Out[10]: '(0, 1]'
    

    The former creates a list, not an interval. The latter isn't even valid Python.

  2. srepr(S.Integers) (and probably others)

    In [11]: srepr(S.Integers)
    Out[11]: 'Integers()'
    

    Integers isn't a name that is imported from sympy. It should print as S.Integers. The str printers should probably do the same.

  3. str(Union)

    In [18]: str(Union(S.Integers, FiniteSet(pi))) 
    Out[18]: 'Integers() U {pi}'
    

    It's not valid Python. It should print as Union(S.Integers, FiniteSet(pi)). Printing as Union(S.Integers, {pi}) is fine when sympify(set) should give FiniteSet #10654 gets merged.

There are likely others. I didn't check too much. An audit of the printing in the sets module would be worthwhile.

@asmeurer asmeurer added Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. printing sets labels Feb 23, 2016
@parvparkhiya
Copy link

I would like to work on this.

@SalilVishnuKapur
Copy link
Contributor

@asmeurer Regarding the issue 1 . I find it outputs something else on the sympy live shell . Kindly have a look .

str(Interval(0, 1, False))
[0,1]

str(Interval(0, 1, True))
(0,1]

Also,

type(str(Interval(0, 1, False)))
<type′str′>

type(str(Interval(0, 1, true)))
<type′str′>

@AnishShah
Copy link
Contributor

The output is the same on sympy live

@SalilVishnuKapur
Copy link
Contributor

@AnishShah but here have a look at this screenshot of what i tried on sympy live.
image

If I am going wrong somewhere then kindly guide.

@AnishShah
Copy link
Contributor

I'm sorry if I'm missing something, but the output in your screenshot is same as the output mentioned by @asmeurer. I don't see any difference.

@SalilVishnuKapur
Copy link
Contributor

'[0, 1]' and [0, 1] . I think it is already a string after getting evaluated by str.

@asmeurer
Copy link
Member Author

@SalilVishnuKapur that's because the SymPy Live shell renders the output as LaTeX.

You should work locally against the git master. SymPy Live has some differences against the normal SymPy which might confuse, but more importantly, it runs SymPy 0.7.6, whereas you want to work against the git master.

@theaverageguy
Copy link

I would like to take this up. :)

@srajangarg srajangarg mentioned this issue Mar 1, 2016
@wrat
Copy link
Contributor

wrat commented Mar 4, 2016

I also want to work on this but i am new here so how should start?

@asmeurer
Copy link
Member Author

asmeurer commented Mar 4, 2016

It looks like this has already been started at #10708, so you should at least wait until that is merged and see if anything is left to do then.

@asmeurer
Copy link
Member Author

asmeurer commented Mar 4, 2016

Also see https://github.com/sympy/sympy/wiki/Development-workflow for general instructions on how to contribute.

@wrat
Copy link
Contributor

wrat commented Mar 4, 2016

Ohkk thanks Aaron Meurer!!! I will wait for that or I will work on other Issue. Thanks for suggestion.

@asmeurer
Copy link
Member Author

Related #10035

@SagarB-97
Copy link
Contributor

@asmeurer
Please review PR #12112

@Upabjojr
Copy link
Contributor

#12213

@asmeurer
Copy link
Member Author

@Upabjojr wrong number? I don't see how that's related.

@Upabjojr
Copy link
Contributor

wrong number? I don't see how that's related.

Sorry, I wanted to link the issue of the And and Or str() printer.

@SagarB-97
Copy link
Contributor

Please review #12112

@SagarB-97
Copy link
Contributor

@Upabjojr I don't think this issue should be closed. #12112 takes care of only str printer. srepr printer is still the same.

@Upabjojr Upabjojr reopened this Apr 3, 2017
@Upabjojr
Copy link
Contributor

Upabjojr commented Apr 3, 2017

@SagarB-97 github apparently closed the issue automatically, I didn't actually notice that.

AnimeshSinha1309 added a commit to AnimeshSinha1309/sympy-repo that referenced this issue Dec 14, 2018
Fixing sympy#10672, fixes printing of S.Integers, S.Reals, S.Naturals and S.Naturals0 in both str() and srepr().
@AnimeshSinha1309
Copy link
Contributor

Is this the expected behaviour of str() and srepr():

>>> from sympy import S
>>> from sympy import srepr
>>> print(S.Integers)
S.Integers
>>> S.Naturals0
S.Naturals0
>>> srepr(S.Naturals)
'S.Naturals'
>>> str(S.Reals)
'S.Reals'

I implemented this but then did not pass the tests as the examples and tests are written for the previous output. Please tell me if this is what was desired so I can fix the tests and examples for the new output.

@asmeurer
Copy link
Member Author

It looks like points 1 and 3 are now fixed. It is no longer necessary to print the fancy sets with S. because they are included in from sympy import * now.

However, this is still wrong:

>>> srepr(Integers)
Integers()

because Integers() raises a TypeError. It should just return "Integers", the same as str.

@AnimeshSinha1309
Copy link
Contributor

AnimeshSinha1309 commented Dec 15, 2018

Thanks, will fix srepr().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Easy to Fix This is a good issue for new contributors. Feel free to work on this if no one else has already. printing sets
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants