search

sloria / konch

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

Load .konchrc via exec. #112

Closed brl0 closed 5 years ago

brl0 commented 5 years ago

Everything seems to work properly, passes all tests, but flake8 says the use_file function is too complex.

sloria commented 5 years ago

This would be quite a breaking change, but more importantly, it is not ideal for konch.py's namespace to leak into .konchrc. Can we avoid this?

brl0 commented 5 years ago

I can understand not wanting to make a breaking change, I am sure that can be avoided.

I am confused and don't fully understand the point about the namespaces. In particular, it seems like the config loading process relies on a shared global namespace to pass the _cfg and _config_registry, which happens to be unexpectedly isolated when executed through python command lines. That is what I thought I learned through testing with the exec approach.

I'll see what I can come up with. I am happy to keep hacking away at this, this is a learning process for me. Let me know if you have any thoughts, suggestions, guidance etc, I am eager to learn.

Thanks for your patience.

brl0 commented 5 years ago

The problem with not using a shared namespace is we are left to guess what namespace the config is loaded into. In # 106, I had previously assumed it would be in konch but you pointed out the config function could be loaded directly or otherwise renamed.

I wonder if #106 or something similar should be used, but add a better error message if konch is not found, and document the recommended method of calling config is through konch.

sloria commented 5 years ago

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