Skip to content
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

Fix IPython requirement #1263

Merged
merged 2 commits into from
Mar 27, 2018
Merged

Conversation

alanhuang122
Copy link
Contributor

IPython 6.0+ requires Python 3.3+, so only install that version if we're using Python 3.3+.

This fixes #1194.

ipython 6.0+ requires python 3.3+
@dgw dgw added this to the 6.5.2 milestone Mar 22, 2018
@dgw
Copy link
Member

dgw commented Mar 22, 2018

Looks good, though the ipython module is actually excluded from tests so… lol.

That said, while this shouldn't hurt anything, I don't think it's actually necessary. Installing IPython with my local pip2 installed IPython 5.5.0, just as it should, because 6.x doesn't support py2. pip is generally smart enough to avoid installing incompatible package versions.

@alanhuang122 Can you think of a good reason to have these IPython requirements in requirements.txt, and also have IPython required in dev-requirements.txt? That just seems redundant. Pay no attention to the drowsy dgw behind the curtain.

@alanhuang122
Copy link
Contributor Author

Yeah, I'd realized after opening this that builds were still succeeding even without this explicit specification. Not sure, in that case, what's going on with the issue.

There is no IPython requirement currently in requirements.txt, unless I'm blind.

@dgw
Copy link
Member

dgw commented Mar 22, 2018

You're not blind, I'm just GitHubbing tired (which one should never do, but I do anyway on a regular basis). It's not there. Which is strange, because it's a runtime dependency, not a dev dependency.

Maybe take this patch and put the requirement where it belongs, and remove it from dev-requirements?

At some point it might be worth considering making niche stuff like IPython (both the requirement and the module, if possible) only install if specifically requested using the extra requirements feature of python (e.g. you'd need to pip install sopel[with_ipython] to get it), but that's a bigger project.

@dgw dgw mentioned this pull request Mar 23, 2018
@dgw dgw added the Bug Things to squish; generally used for issues label Mar 23, 2018
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

@dgw dgw merged commit 608caf8 into sopel-irc:master Mar 27, 2018
@alanhuang122 alanhuang122 deleted the ipython-requirements branch April 6, 2018 06:48
@dgw dgw added Bugfix Generally, PRs that reference (and fix) one or more issue(s) and removed Bug Things to squish; generally used for issues labels Jan 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix Generally, PRs that reference (and fix) one or more issue(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot install dev dependencies on Python 2.7
2 participants