Fix IPython requirement for Python 2

rkern / line_profiler

(OLD REPO) Line-by-line profiling for Python - Current repo ->

https://github.com/pyutils/line_profiler

Other

3.61k stars 255 forks source link

Fix IPython requirement for Python 2 #90

Closed thanatos closed 5 years ago

thanatos commented 7 years ago

IPython recently released IPython 6.0.0, which dropped support for Python 2. Modify setup.py to depend on IPython versions old than 6.0.0 for Python 2 (i.e., those versions which still support Python 2); keep the behavior unchanged for Python 3.

Fixes #89

rkern commented 7 years ago

IPython should not have been made a requirement. That would be the thing to fix.

thanatos commented 7 years ago

I'm not intimately familiar with the codebase; that's a bigger change than I was looking to make. Are you saying that it could/should be torn out? (Wouldn't that break anyone depending on it?)

(My aim was simply to get the project functional under Python 2, again, not to add/remove features.)

rkern commented 7 years ago

The functionality should be available for people who are using IPython but not unconditionally required for people who are not using IPython. I.e. there should be no imports from the IPython package at the top level. This used to be the case when the %lprun magic was defined by a function rather than a class.

thanatos commented 7 years ago

If I get some more free time, I'll certainly consider that. Nonetheless, I think this PR is still a step in the right direction; presently, the package is broken for Python 2 users (by way of having a dependency on a Python3-only import) and that having the package install at all would be more pragmatic than trying to fix a subfeature to work conditionally in the short-term.

Edit: so, it seems that the latest pip and PyPI may include a signal to not install the latest IPython, despite the requirements section somewhat signalling that, and might do the right thing. I'll have to re-evaluate if that could work for us (we'd need to ensure pip is upgraded everywhere…) In this manner, this PR may be obsolete.