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

init_session(auto_symbols=True) and init_session(auto_int_to_Integer=True) do not work #2839

Closed
asmeurer opened this Issue Jan 26, 2014 · 12 comments

Comments

Projects
None yet
4 participants
@asmeurer
Copy link
Member

asmeurer commented Jan 26, 2014

In [5]: init_session(auto_int_to_Integer=True, auto_symbols=True)
IPython console for SymPy 0.7.4.1-git (Python 3.3.3-64-bit) (ground types: python)

These commands were executed:
>>> from __future__ import division
>>> from sympy import *
>>> x, y, z, t = symbols('x y z t')
>>> k, m, n = symbols('k m n', integer=True)
>>> f, g, h = symbols('f g h', cls=Function)

Documentation can be found at http://www.sympy.org

In [6]: 1/2
Out[6]: 0.5

In [7]: notdefined
---------------------------------------------------------------------------
NameError                                 Traceback (most recent call last)
<ipython-input-7-6ee7e72db811> in <module>()
----> 1 notdefined

NameError: name 'notdefined' is not defined

isympy -I works.

We need to convert these to use the new IPython ast machinery.

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Jan 26, 2014

@takluyver could you point me to the docs for the new ast stuff? Also, do you remember if I or you ever figured out a working example for undefined names. I seem to remember that one was nontrivial to get right?

@takluyver

This comment has been minimized.

Copy link

takluyver commented Jan 27, 2014

Sure, the docs are here: http://ipython.org/ipython-doc/stable/config/inputtransforms.html#ast-transformations

I don't think we ever wrote an actual example. I think it's fairly straightforward if you don't need to support names first appearing within function definitions:

  • Scan the code for Name nodes with ctx=Del().
  • Implement visit_FunctionDef and visit_ClassDef as no-ops so we don't scan code in those.
  • Check names found against user_ns.
  • For any that don't already exist, insert AST nodes equivalent to foo = symbol('foo') at the top of the cell.
  • If the variable is defined in that cell, then that definition comes after the one we just added, so the variable is redefined to the value the user expects.

Supporting variables in function definitions is a bit trickier, because you presumably don't want to leak those definitions out to the global scope. But it should be feasible to keep a stack of references to function definitions, and insert the foo = symbol('foo') into the function at the top of the stack.

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Jan 28, 2014

I think it's fine to not support functions.

I was worried about names that are defined within the block, but I just realized that this doesn't matter. It will just end up being a redefinition instead. Creating symbols is cheap and side-effect free, so there's no issues there.

Why ctx=Del()? I don't understand why it shouldn't be ctx=Load().

@takluyver

This comment has been minimized.

Copy link

takluyver commented Jan 28, 2014

Yep, I had the same worry and the same realisation.

It should be ctx=Load(), I evidently had a brainfart. ;-)

@jcrist

This comment has been minimized.

Copy link
Member

jcrist commented Apr 21, 2015

Copying this over from #2921, and then closing that issue (duplicate):

Quote:
"TODO from sympy/interactive/session.py (commented out code, deleted in #2219):

# XXX: Something like this might be used, but it only works on single line
# inputs.  See
# http://mail.scipy.org/pipermail/ipython-user/2012-August/010846.html and
# https://github.com/ipython/ipython/issues/1491.  So instead we are forced to
# just monkey-patch run_cell until IPython builds a better API.
 class IntTransformer(object):
     """
     IPython command line transformer that recognizes and replaces int
     literals.

     Based on
     https://bitbucket.org/birkenfeld/ipython-physics/src/71b2d850da00/physics.py.

     """
     priority = 99
     enabled = True
     def transform(self, line, continue_prompt):
         import re
         from tokenize import TokenError
         leading_space = re.compile(' *')
         spaces = re.match(leading_space, line).span()[1]
         try:
             return ' '*spaces + int_to_Integer(line)
         except TokenError:
             return line

 int_transformer = IntTransformer()

 def enable_automatic_int_sympification(app):
     """
     Allow IPython to automatically convert integer literals to Integer.

     This lets things like 1/2 be executed as (essentially) Rational(1, 2).
     """
     app.shell.prefilter_manager.register_transformer(int_transformer)

"

@takluyver

This comment has been minimized.

Copy link

takluyver commented Apr 21, 2015

As described above, we did build a better API - AST transformations. There's an example of how it works in the tests:

https://github.com/ipython/ipython/blob/411cd57051173af49a54f55bfce9c22198bc43e8/IPython/core/tests/test_interactiveshell.py#L616

@asmeurer asmeurer referenced this issue Apr 14, 2016

Merged

Architecture #36

skirpichev added a commit to skirpichev/diofant that referenced this issue May 17, 2016

skirpichev added a commit to skirpichev/diofant that referenced this issue Nov 2, 2016

Add regression tests & mention closed issues
    close sympy/sympy#3112 (MrvAsympt was added in diofant#6)
    close sympy/sympy#9173 (test was added in 5a510ac)
    close sympy/sympy#9808 (fixed in 09e539b)
    close sympy/sympy#9341 (fixed in af98a00)
    close sympy/sympy#9908 (fixed in cc3fa8d)
    close sympy/sympy#6171 (test added in d278031)
    close sympy/sympy#9276 (diagnose_imports.py removed in ab8c535)
    close sympy/sympy#10201 (fixed in 0d0fc5f)
    close sympy/sympy#9057 (test was added in 8290a0c)
    close sympy/sympy#11159 (test was added in ffb76cb)
    close sympy/sympy#2839 (new AST transformers are used, see diofant#278 and diofant#167)
    close sympy/sympy#11081 (see ed01e16 and bb92329)
    close sympy/sympy#10974 (see 73fc425)
    close sympy/sympy#10806 (test in 539929a)
    close sympy/sympy#10801 (test in 2fe3da5)
    close sympy/sympy#9549 (test in 88bdefa)
    close sympy/sympy#4231 (test was added in fb411d5)
    close sympy/sympy#8634 (see 2fcbb58)
    close sympy/sympy#8481 (see 1ef20d3)
    close sympy/sympy#9956 (fixed in a34735f)
    close sympy/sympy#9747 (see e117c60)
    close sympy/sympy#7853 (see 3e4fbed)
    close sympy/sympy#9634 (see 2be03f5)
    close sympy/sympy#8500 (fixed in diofant#104 and finally in diofant#316)
    close sympy/sympy#9192 (see 9bf622f)
    close sympy/sympy#7130 (see e068fa3)
    close sympy/sympy#8514 (see b2d543b)
    close sympy/sympy#9334 (see 90de625)
    close sympy/sympy#8229 (see 9755b89)
    close sympy/sympy#8061 (see 7054f06)
    close sympy/sympy#7872 (tested in diofant#6)
    close sympy/sympy#3496 (tested in test_log_symbolic)
    close sympy/sympy#2929 (see da7db7a)
    close sympy/sympy#8203 (oo is not a real, see diofant#36)
    close sympy/sympy#7649 (0 is imaginary since diofant#8)
    close sympy/sympy#7256 (fixed in c0a4549)
    close sympy/sympy#6783 (see cb28d63)
    close sympy/sympy#5662 (is_integer issue fixed in 6bfa9f8, there is no is_bounded anymore)
    close sympy/sympy#5295 (fixed with diofant#354)
    close sympy/sympy#4856 (we now have flake/pep tests)
    close sympy/sympy#4555 (flake8 enabled after diofant#214)
    close sympy/sympy#5773 (cmp_to_key removed after diofant#164 and c9acbf0)
    close sympy/sympy#5484 (see above)

    Added regression tests:
    from https://groups.google.com/forum/#!topic/sympy/LkTMQKC_BOw
    fixes sympy/sympy#8825 (probably via diofant#209)
    fixes sympy/sympy#8635
    fixes sympy/sympy#8157
    fixes sympy/sympy#7872
    fixes sympy/sympy#7599
    fixes sympy/sympy#6179
    fixes sympy/sympy#5415
    fixes sympy/sympy#2865
    fixes sympy/sympy#5907
    fixes sympy/sympy#11722

    Closes diofant#347

skirpichev added a commit to skirpichev/diofant that referenced this issue Nov 2, 2016

Add regression tests & mention closed issues
    close sympy/sympy#3112 (MrvAsympt was added in diofant#6)
    close sympy/sympy#9173 (test was added in 5a510ac)
    close sympy/sympy#9808 (fixed in 09e539b)
    close sympy/sympy#9341 (fixed in af98a00)
    close sympy/sympy#9908 (fixed in cc3fa8d)
    close sympy/sympy#6171 (test added in d278031)
    close sympy/sympy#9276 (diagnose_imports.py removed in ab8c535)
    close sympy/sympy#10201 (fixed in 0d0fc5f)
    close sympy/sympy#9057 (test was added in 8290a0c)
    close sympy/sympy#11159 (test was added in ffb76cb)
    close sympy/sympy#2839 (new AST transformers are used, see diofant#278 and diofant#167)
    close sympy/sympy#11081 (see ed01e16 and bb92329)
    close sympy/sympy#10974 (see 73fc425)
    close sympy/sympy#10806 (test in 539929a)
    close sympy/sympy#10801 (test in 2fe3da5)
    close sympy/sympy#9549 (test in 88bdefa)
    close sympy/sympy#4231 (test was added in fb411d5)
    close sympy/sympy#8634 (see 2fcbb58)
    close sympy/sympy#8481 (see 1ef20d3)
    close sympy/sympy#9956 (fixed in a34735f)
    close sympy/sympy#9747 (see e117c60)
    close sympy/sympy#7853 (see 3e4fbed)
    close sympy/sympy#9634 (see 2be03f5)
    close sympy/sympy#8500 (fixed in diofant#104 and finally in diofant#316)
    close sympy/sympy#9192 (see 9bf622f)
    close sympy/sympy#7130 (see e068fa3)
    close sympy/sympy#8514 (see b2d543b)
    close sympy/sympy#9334 (see 90de625)
    close sympy/sympy#8229 (see 9755b89)
    close sympy/sympy#8061 (see 7054f06)
    close sympy/sympy#7872 (tested in diofant#6)
    close sympy/sympy#3496 (tested in test_log_symbolic)
    close sympy/sympy#2929 (see da7db7a)
    close sympy/sympy#8203 (oo is not a real, see diofant#36)
    close sympy/sympy#7649 (0 is imaginary since diofant#8)
    close sympy/sympy#7256 (fixed in c0a4549)
    close sympy/sympy#6783 (see cb28d63)
    close sympy/sympy#5662 (is_integer issue fixed in 6bfa9f8, there is no is_bounded anymore)
    close sympy/sympy#5295 (fixed with diofant#354)
    close sympy/sympy#4856 (we now have flake/pep tests)
    close sympy/sympy#4555 (flake8 enabled after diofant#214)
    close sympy/sympy#5773 (cmp_to_key removed after diofant#164 and c9acbf0)
    close sympy/sympy#5484 (see above)

    Added regression tests:
    from https://groups.google.com/forum/#!topic/sympy/LkTMQKC_BOw
    fixes sympy/sympy#8825 (probably via diofant#209)
    fixes sympy/sympy#8635
    fixes sympy/sympy#8157
    fixes sympy/sympy#7872
    fixes sympy/sympy#7599
    fixes sympy/sympy#6179
    fixes sympy/sympy#5415
    fixes sympy/sympy#2865
    fixes sympy/sympy#5907
    fixes sympy/sympy#11722

    Closes diofant#347
@mhlr

This comment has been minimized.

Copy link
Contributor

mhlr commented Feb 8, 2017

I have found a workaround for this. The problem is that when in an already running IPython session, init_session does not call init_ipython_session which is responsible for enabling the options as well as starting the session. That call can be made explicitly first:

sympy.interactive import init_session
from sympy.interactive.session import init_ipython_session
init_ipython_session(auto_symbols=True, auto_int_to_Integer=True)
init_session(ipython=None, auto_symbols=True, auto_int_to_Integer=True)

See http://stackoverflow.com/a/42105135/843348

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Feb 8, 2017

@mhlr so I guess the if ip is None check before it calls init_ipython_session should be removed. Are there downsides to calling it twice? I tested your workaround and it seems to work. Can you send a patch?

@mhlr

This comment has been minimized.

Copy link
Contributor

mhlr commented Feb 9, 2017

@asmeurer I do not know if there are downsides. I have not observed any, but this is the first time I have really looked inside sympy source and I have only started using this method.
There is still the problem that the initialization need to be done separately, eg doing eval-buffer from emacs does not work.

I am happy to make a patch if you think this is good.

@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Feb 9, 2017

Yes, please send a patch.

@mhlr

This comment has been minimized.

Copy link
Contributor

mhlr commented Feb 9, 2017

@asmeurer Ok. So I will just remove the if statement.

Also do you think wrapping the initialization in an IPython magic would fix the problem with needing separate evaluations?

mhlr added a commit to mhlr/sympy that referenced this issue Feb 9, 2017

call `init_ipython_session` even if in IPython
This is to fix sympy#2839 since `init_ipython_session` is responsible for enabling the options.

mhlr added a commit to mhlr/sympy that referenced this issue Feb 9, 2017

call `init_ipython_session` even if in IPython
This is to fix sympy#2839 since `init_ipython_session` is responsible for enabling the options.
@asmeurer

This comment has been minimized.

Copy link
Member

asmeurer commented Feb 9, 2017

I don't know. Having a magic command sounds useful either way, though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment