remove install_ext#8763
Conversation
|
OK, I've checked the tests in the IPython folder and didn't find any calls to install_ext in them, so I'm not sure where that call travis is making is or how to remove it. Working on it. |
|
Right! I missed that one. There is a extension test function that depended on install_ext. I've commented it out instead of deleting it just in case someone needs to reuse that code, since it didn't seem totally expendable. And sorry about the mess with the rst files, took me a while to figure those out. |
|
What is the replacement story? Build a package for each 10-liner? Would it be possible to add a ipython-contrib/ipython-extensions repo (with some guidelines what to add), which is released every so often? |
Yes, it's not that hard : https://github.com/Carreau/pip_magic/blob/master/pip_magic/__init__.py Package with flit that make publishing packages really easy.
ipython-contrib is not an official IPython repo, but I thing the people there wouldn't say no. It would be released as often as the maintainers there would release, we could even set up travis to release really often. |
|
@Achifaifa we are now on the 5.0 branch, do you want to rebase, so that we can merge. You also directly delete the test instead of just commenting. |
|
Oops, missed the notification and didn't read this for some time. |
| _ip.magic("unload_ext daft_extension") | ||
| assert 'arq' not in _ip.user_ns | ||
| finally: | ||
| sys.path.remove(daft_path) |
There was a problem hiding this comment.
Why delete this test? It doesn't seem to use install_ext at all.
|
I removed the function I commented in a previous commit but apparently someone edited it somewhere along the way to not use install_ext, sorry about that. |
|
Aha, gotcha. The code as it stands reverts the test to the version that did use |
|
Oh great, thank you. I've been trying to do it myself but I've just been running around like a chicken in detached head mode :( |
No problem, we have plenty of time.
That SO was likely me to avoid deprecation warnings of using install ext :-( @takluyver said :
FYI that this is because we attempt to avoid merge from the master branch usually, |
|
Merged as d953069 . Thanks @Achifaifa! |
Deleted the function, checked for function references through code and tests (nothing found) and added information of the changes to the /docs/source/whatsnew/pr/ folder.
Not sure if the format used in the change docs is the correct one, the examples I checked were empty files, please check that before merging.
closes #8634