sloria / konch

Configures your Python shell.
https://konch.readthedocs.io/
MIT License
406 stars 18 forks source link

Issue with empty context #99

Closed brl0 closed 5 years ago

brl0 commented 5 years ago

I have multiple python environments and often prefer to run packages by specifying which python instance I would like to run under by running 'python -m cmd'. This works great.

I encountered an issue with Konch I was not able to easily correct. When running Konch in such a manner, the .konchrc file is processed, the setup() function is executed, but the context is not retained, and instead a blank context is used. It seems to be a scoping issue.

I was able to get it working properly by specifically setting '_cfg = mod.konch._cfg'. However, this seems to have upset mypy, so I don't know if this is an appropriate solution.

Let me know if I can be of further assistance.

brl0 commented 5 years ago

I believe I have a workable solution to this issue, I just can't figure out how to create a second pull request with my first pull request pending. I also haven't figured out if I can simply cancel the first pull request. But this issue should be resolved with my next PR. Sorry for any difficulty caused, this is my first attempt at contributing and I am still learning how the process should work.

brl0 commented 5 years ago

Addressed in my second attempt at a pull request.

sloria commented 5 years ago

An issue should only be closed if it is fixed on master (i.e. if your PR fix gets merged), so re-opening.

brl0 commented 5 years ago

Thanks for the feedback on this and the PR, I will give that a try. I don't know how to break up or manage the changes or even if it is possible within git, I am lacking in my ninGitsu skills. I will just redo the requested changes and resubmit as multiple PR.

sloria commented 5 years ago

You can just create multiple branches off of master

git checkout master
git checkout -b fix-1
# ...commit your changes and send pull request

git checkout master
git checkout -b another-fix
# ...commit changes for the other thing
brl0 commented 5 years ago

Running konch from local copy reproduces the blank context behavior. python konch.py Adding '-d' debugging switch clearly shows the issue: DEBUG konch.py: Updating with {'context': [<module 'konch' from '/mnt/data/source/repos/konch/konch.py'>, <function env at 0x7f45fe9181e0>, <class 'pathlib.Path'>, <module 'IPython' from '/opt/conda/envs/py36/lib/python3.6/site-packages/IPython/__init__.py'>, <module 'bpython' from '/opt/conda/envs/py36/lib/python3.6/site-packages/bpython/__init__.py'>, <module 'ptpython' from '/opt/conda/envs/py36/lib/python3.6/site-packages/ptpython/__init__.py'>], 'prompt': '[konch] >>> ', 'ipy_colors': 'linux', 'ipy_autoreload': True, 'ptpy_vi_mode': True, 'context_format': 'full'} DEBUG konch.py: Starting with config {'context': {}, 'banner': None, 'shell': <class '__main__.AutoShell'>, 'prompt': None, 'output': None, 'context_format': 'full'} Notice that the first entry shows the context properly being updated. The second entry shows that it is starting with an empty context.

brl0 commented 5 years ago

I thought it would be good to share a quick update. This problem seems to be a scoping issue. One approach I was trying was to grab the context from the imported .konchrc module and put that directly into the global scope, hence the get_config function. However, my initial attempt at this approach did not properly account for named contexts. I believe this is why some tests failed in CI which were being skipped on my system. I have not yet spent enough time to find the best way to ensure the correct imported context is always handed off. I am open to any suggestions on things to try.

sloria commented 5 years ago

It might be worth evaluating how IPython and ptpython load their config files (e.g. ~/.ptpython/config.py), since their use case are so similar.

brl0 commented 5 years ago

A quick test with this: import konch as k k.config({"context": [k]})

konch is imported into the context as konch not as k as one would expect.

brl0 commented 5 years ago

It might be worth evaluating how IPython and ptpython load their config files (e.g. ~/.ptpython/config.py), since their use case are so similar.

Good idea, thanks. IPython uses Traitlets. Here is how ptpython does it:

    # Run the config file in an empty namespace.
    try:
        namespace = {}

        with open(config_file, 'rb') as f:
            code = compile(f.read(), config_file, 'exec')
            six.exec_(code, namespace, namespace)

        # Now we should have a 'configure' method in this namespace. We call this
        # method with the repl as an argument.
        if 'configure' in namespace:
            namespace['configure'](repl)
sloria commented 5 years ago

IPython uses traitlets, but I'm not sure that's relevant. I suggest looking into how config files get loaded when running with python -m (is this supported in IPython?).

The exec approach in ptpython is interesting; I wonder if it would work with .konchrc. 🤔 Would you like to try it out?

brl0 commented 5 years ago

IPython uses traitlets, but I'm not sure that's relevant.

Possible relevance, from description:

The package also includes a mechanism to use traitlets for configuration, loading values from files or from command line arguments.

Reference from source: from traitlets.config.loader import ConfigFileNotFound, PyFileConfigLoader

Example snippet

This is not something I am suggesting, just trying to illustrate how they handle importing the config. I think part of Konch's beauty is the relative simplicity of creating and managing sophisticated sets of configs, which I would hope to not complicate.

I will give the ptpython approach a shot. Let me know if you have any other thoughts or suggestions.

brl0 commented 5 years ago

Also, a quick mention about load_module:

Deprecated since version 3.4: The recommended API for loading a module is exec_module() (and create_module()).

sloria commented 5 years ago

Oh, I see--I didn't realize traitlets was also used to load files.

Also good to know about the load_module deprecation--I missed that. Will fix when I have some time. Thanks for looking into these!

brl0 commented 5 years ago

I think testing with exec helped illuminate the issue.

As an experiment, I removed import konch from .konchrc, and changed the call konch.config to config. Then run exec(code, globals(), locals()) to load the configs, and it worked.

The issue now is that since .konchrc is no longer loading the file as a module, setup and teardown are not being run. I can get around this by creating new global vars, _setup and _teardown.

This seems to work, but need to run tests etc to be sure I didn't break anything.

sloria commented 5 years ago

See https://github.com/sloria/konch/pull/113#issuecomment-504709956; the preferred solution is to either use programmatically

# myproject/shell.py
import konch

konch.main()
python shell.py <args>

or use a setuptools command: https://konch.readthedocs.io/en/latest/#integrating-with-setuptools-python-setup-py-shell

brl0 commented 5 years ago

Thanks for the workaround. This makes a useable one-liner: python -c 'import konch; konch.main()'