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

Improvements to IPython printing: strings, floats, and ints. #2683

Merged
merged 6 commits into from Jan 3, 2014

Conversation

moorepants
Copy link
Member

Removed strings from LaTeX printing in IPython as this was causing dictionaries
that contained strings and other valid LaTeX printable types to render. This is
typically not desirable especially if you have a dictionary with only strings,
floats, and integers.

I also added an option print_builtin to init_printing that allows you to
disable the float and integer LaTeX printing. For example, if you don't want a
dictionary of integer keys and float values to render in LaTeX.

Fixes issue #2672.

Tasks

  • Add tests to check that the LaTeX printing is correct.

Removed strings from LaTeX printing in IPython as this was causing dictionaries
that contained strings and other valid LaTeX printable types to render. This is
typically not desirable especially if you have a dictionary with only strings,
floats, and integers.

I also added an option `print_builtin` to `init_printing` that allows you to
disable the float and integer LaTeX printing. For example, if you don't want a
dictionary of integer keys and float values to render in LaTeX.

Fixes issue sympy#2672.
@moorepants
Copy link
Member Author

@moorepants
Copy link
Member Author

The Travis failure seems unrelated to the PR.

@asmeurer
Copy link
Member

Add tests.

@asmeurer
Copy link
Member

I think you somehow squashed on top of #2656.

@moorepants
Copy link
Member Author

With regards to the failure? What do you mean I squashed on top of #2656?

@moorepants
Copy link
Member Author

How do I check for LaTeX printing in the IPython notebook? It seems that my test is only checking for pretty printing.

@asmeurer
Copy link
Member

Your first commit contains changes from that PR.

@asmeurer
Copy link
Member

I guess you need to check whatever the mime type of mathjax is instead of text/plain.

@moorepants
Copy link
Member Author

My first commit only has things I added as far as I can tell. I'm not seeing what you think is from #2656

@moorepants
Copy link
Member Author

I don't really understand what's going on in the tests. It looks like we call format() on an object and then see what it returns. This returns a dictionary with various types of output? I'm only seeing a text/plain output. What should I call on the object to force a mimetype that matches mathjab output?

@asmeurer
Copy link
Member

The docstring of init_printing.

@moorepants
Copy link
Member Author

I edited the doc string of init_printing because I noticed that many of the options were not documented.

@asmeurer
Copy link
Member

Oh, so it was made independently. In that case, maybe you could review the said PR, as it has many of the same changes you are making.

@moorepants
Copy link
Member Author

I think the only same changes in #2656 are the addition of the docstrings for the extra parameters. One of our commits has to go in first and the other adjust. I think this one is ready to go. I couldn't tell the status of the other one. @moble can you comment? I can just remove these doc string additions, but we are likely to have a merge conflict regardless.

@asmeurer
Copy link
Member

I don't remember how. I thought you just went to https://www.hipchat.com/ghSp7E1uY and entered your name.

@moorepants
Copy link
Member Author

I've added the last test here to check for the LaTeX representations. Ready to be reviewed for merge.

@moorepants
Copy link
Member Author

@asmeurer ping

@asmeurer
Copy link
Member

Things to check:

  • both the notebook and the qtconsole
  • Python 2 and Python 3
  • older versions of IPython (you don't have to go back too far, maybe one major version).

@moorepants
Copy link
Member Author

Here is the qtconsole:

ipython_003

@moorepants
Copy link
Member Author

I don't have environments on my local machine setup to test all kinds of versions of Python and IPython. I've been meaning to get familiar with conda, but haven't had the chance. Have people checked all these other versions before me? Is there no automated testing to deal with this?

@asmeurer
Copy link
Member

conda/Anaconda would be the easiest way to do this. You could check everything in a jiffy. I personally can't check everything because there is not yet a pyqt (hence qtconsole) conda package for Python 3 on Mac OS X, but I believe there is on Linux.

@moorepants
Copy link
Member Author

There doesn't seem to be Python 3 packages for conda for IPython 0.13.2. This is a lot of fussing around...not really a jiffy.

@moorepants
Copy link
Member Author

Here is Python 2.7.6, IPython 0.13.2 in the notebook:

selection_004

@moorepants
Copy link
Member Author

Python 3.3, Ipython 1.1 in the notebook:

selection_005

@moorepants
Copy link
Member Author

I can't get qtconsole to run with anaconda. I keep getting failures in loading pygments.lexers.

@moorepants
Copy link
Member Author

I've checked everything you listed except for all the permutations of version, of course.

@asmeurer
Copy link
Member

Also check the tests. I can't remember if they are run in Travis.

But anyway, +1.

@moorepants
Copy link
Member Author

The tests only run if IPython is installed. So, I don't think they happen on Travis.

I'm getting a test failure with Python 3.3 and IPython 0.13.2:

(sympy-3.3-testing)moorepants@moorepants-2170p:sympy(ipython-printing)$ bin/test sympy/interactive/tests/
================================================================ test process starts =================================================================
executable:         /home/moorepants/envs/sympy-3.3-testing/bin/python  (3.3.3-final-0) [CPython]
architecture:       64-bit
cache:              yes
ground types:       python 
random seed:        64048801
hash randomization: on (PYTHONHASHSEED=3531650678)

sympy/interactive/tests/test_interactive.py[1] .                                                                                                  [OK]
sympy/interactive/tests/test_ipython.py[2] ..                                                                                                     [OK]
sympy/interactive/tests/test_ipythonprinting.py[2] .F                                                                                           [FAIL]

______________________________________________________________________________________________________________________________________________________
_____________________________________ sympy/interactive/tests/test_ipythonprinting.py:test_print_builtin_option ______________________________________
  File "/home/moorepants/src/sympy/sympy/interactive/tests/test_ipythonprinting.py", line 66, in test_print_builtin_option
    assert text == "{pi: 3.14, n_i: 3}"
AssertionError

================================================ tests finished: 4 passed, 1 failed, in 1.46 seconds =================================================
DO *NOT* COMMIT!

This is returning the unicode version instead of a plain string. It passes on master but fails on this branch. I didn't touch that test. Not sure what the issue is.

@moorepants
Copy link
Member Author

Would this failure have to do with removing the 'string_types'?

@moorepants
Copy link
Member Author

No, this is something I wrote. Maybe it has to do with Python 3.3 strings being unicode.

@moorepants
Copy link
Member Author

This is interesting:

(sympy-3.3-testing)moorepants@moorepants-2170p:sympy(ipython-printing)$ ipython --pdb
Python 2.7.5+ (default, Sep 19 2013, 13:48:49) 
Type "copyright", "credits" or "license" for more information.

IPython 1.1.0 -- An enhanced Interactive Python.
?         -> Introduction and overview of IPython's features.
%quickref -> Quick reference.
help      -> Python's own help system.
object?   -> Details about 'object', use 'object??' for extra details.

In [1]: from sympy.interactive.tests import test_ipythonprinting

In [2]: test_ipythonprinting.test_ipythonprinting()

In [3]: test_ipythonprinting.test_ipythonprinting()
---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
<ipython-input-3-ee1f64b37974> in <module>()
----> 1 test_ipythonprinting.test_ipythonprinting()

/home/moorepants/src/sympy/sympy/interactive/tests/test_ipythonprinting.pyc in test_ipythonprinting()
     29         assert app.user_ns['a2']['text/plain'] == "pi**2"
     30     else:
---> 31         assert app.user_ns['a'][0]['text/plain'] == "pi"
     32         assert app.user_ns['a2'][0]['text/plain'] == "pi**2"
     33 

AssertionError: 
> /home/moorepants/src/sympy/sympy/interactive/tests/test_ipythonprinting.py(31)test_ipythonprinting()
     30     else:
---> 31         assert app.user_ns['a'][0]['text/plain'] == "pi"
     32         assert app.user_ns['a2'][0]['text/plain'] == "pi**2"

ipdb> app.user_ns['a'][0]['text/plain']
u'\u03c0'

Seems the test passes on the first call but not on the second. Is that because init_printing has bee called on the first call to the test, but never reset?

Update: Notice that this isn't using the conda ipython.

@moorepants
Copy link
Member Author

The tests pass on my machine for Python 3.3/IPython 0.13.2 but fail again on Python 2.7.5/IPython 1.1.0. It is some kind of unicode issue. I'm not familiar at all in the differences in strings/unicode from Python 2 to Python 3. I'll come back to this.

@asmeurer
Copy link
Member

asmeurer commented Jan 3, 2014

Can you paste the error here?

@moorepants
Copy link
Member Author

Ok, tests now pass on:

  • Python 2.7.5 + IPython 1.1.0
  • Python 3.3.3 + IPython 1.1.0
  • Python 3.3.3 + IPython 0.13.2

Gonna merge based on the +1 from @asmeurer.

Should be ready to merge at this point.

moorepants added a commit that referenced this pull request Jan 3, 2014
Improvements to IPython printing: strings, floats, and ints.
@moorepants moorepants merged commit d906a9a into sympy:master Jan 3, 2014
@moorepants moorepants deleted the ipython-printing branch January 8, 2014 22:57
@asmeurer
Copy link
Member

asmeurer commented Jan 9, 2014

Can you document any major changes (here and in other PRs) in the release notes?

@moorepants
Copy link
Member Author

Yes, I'll do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants