Make Human work w/ both prompt_toolkit v1 and v2 #1223
Conversation
As of today, some systems work with prompt_toolkit v1 by default, but Arch Linux has migrated to v2, and it's the last version on PyPI To maintain the Arch Linux package of this library, it was required to revert the 8dba4c0 commit, yet that didn't suffice to make the Human class work in that system This commit changes the Human strategy to work on environments with either prompt_toolkit versions, the main differences are: - Human._history_toolbar no longer return a list with a tuple, but just the raw content (making its tests more straightforward) - self.status_messages["toolbar"] is a function as expected by the prompt_toolkit version available Although the "pygments.toolbar" string is there when using the prompt_toolkit v2, the "pygments" library isn't a dependency, since using the Human strategy works fine in a virtual environment without pygments
| @@ -4,7 +4,7 @@ hypothesis==3.2 | |||
| matplotlib>=2.0.0,<3.0.0 | |||
| numpy>=1.9.2 | |||
| pandas>=0.18.1 | |||
| prompt-toolkit>=1.0.7,<2.0.0 | |||
| prompt-toolkit>=1.0.7 | |||
There was a problem hiding this comment.
This breaks the system install of ipython which is not yet compatible with prompt-toolkit>2.0.0. #1183
Once ipython is using the latest prompt-toolkit we will migrate to just use 2.0.0+.
There was a problem hiding this comment.
IPython already requires v2+: /~https://github.com/ipython/ipython/blob/7.1.1/setup.py#L192
There was a problem hiding this comment.
Actually, the oldest IPython that required prompt_toolkit v1 was the 6.5.0, and the marked line is compatible with it (actually, it's compatible with both versions). /~https://github.com/ipython/ipython/blob/6.5.0/setup.py#L193
There was a problem hiding this comment.
Perhaps my diagnosis of the source of the issue of #1217 and the linked discussions on #1183 isn't correct. Nonetheless the fix in #1212 is preferable as it'll move us to just using prompt_toolkit v2 but any fix needs to not break the common data science tools like jupyter and ipython and (as of 3 minutes ago): updating to the conda distribution of Jupyter currently keeps ipython at 6.5.
|
I'm maintaining the package for this library in the AUR. For the v4.4.0-2 packaged in AUR, I've got this patch and applied it using this: patch -Rp1 < "$srcdir/8dba4c0.patch"As it didn't suffice, I've written this code I'm pulling right now. |
|
Closing this, please reopen if further discussion is required. :) 👍 |
|
Something seem wrong in Travis CI: |
|
Could that be because I closed the PR? Let me reopen to see if that keeps travis happy. |
|
I've just installed jupyter in to a clean environment and it now brings |
Actually, I couldn't install both jupyter and this axelrod library together in a virtual environment, and that was the main reason I've written the PKGBUILD for a system install in Arch Linux. After Arch updated to prompt_toolkit to v2 in the community repo, I had no alternative but to patch some fix. |
|
@danilobellini this looks like the right fix of #1183, however I'd prefer it if we just supported |
|
This is more of a transition PR. I can change this PR to be v2-only, but if the library were mine, I'd prefer to keep both versions during a transition period (e.g. dropping the v1 support only in Axelrod v5.1, or when Ubuntu update to v2), unless it's not possible to keep both. I'll replace the |
Yup, I agree, the v2/v1 stuff here looks good to me. @danilobellini thank you for taking the time to discuss this. Once merged we'll open an issue to document the removal of v1 imports when we're ready to do so. @danilobellini we have a two core developer review rule so @marcharper and/or @meatballs will probably be along when they get a moment :) |
As of today, some systems work with prompt_toolkit v1 by default, but
Arch Linux has migrated to v2, and it's the last version on PyPI
To maintain the Arch Linux package of this library, it was required to
revert the 8dba4c0 commit, yet that didn't suffice to make the Human
class work in that system
This commit changes the Human strategy to work on environments with
either prompt_toolkit versions, the main differences are:
just the raw content (making its tests more straightforward)
prompt_toolkit version available
Although the "pygments.toolbar" string is there when using the
prompt_toolkit v2, the "pygments" library isn't a dependency, since
using the Human strategy works fine in a virtual environment without
pygments