Add requirement for more-itertools that works on python 2

[cheister]

Problem

The latest release of more-itertools (6.0.0) only works on Python 3. https://github.com/erikrose/more-itertools/releases. more-itertools is a requirement of pytest.

When running with python2 I get

  File "/tmp/.pants.d/test/pytest-prep/CPython-2.7.10/15d60b3366ef43c7a4ae6b5f5e2d7567a95fe5f4/.deps/pytest-3.6.4-py2.py3-none-any.whl/pytest.py", line 10, in <module>
    from _pytest.fixtures import fixture, yield_fixture
  File "/tmp/.pants.d/test/pytest-prep/CPython-2.7.10/15d60b3366ef43c7a4ae6b5f5e2d7567a95fe5f4/.deps/pytest-3.6.4-py2.py3-none-any.whl/_pytest/fixtures.py", line 8, in <module>
    from more_itertools import flatten
  File "/tmp/.pants.d/test/pytest-prep/CPython-2.7.10/15d60b3366ef43c7a4ae6b5f5e2d7567a95fe5f4/.deps/more_itertools-6.0.0-py2-none-any.whl/more_itertools/__init__.py", line 1, in <module>
    from more_itertools.more import *  # noqa
  File "/tmp/.pants.d/test/pytest-prep/CPython-2.7.10/15d60b3366ef43c7a4ae6b5f5e2d7567a95fe5f4/.deps/more_itertools-6.0.0-py2-none-any.whl/more_itertools/more.py", line 329
    def _collate(*iterables, key=lambda a: a, reverse=False):

This has been fixed on the latest version of pytest but I'm not sure if we can update to that version or not? pytest-dev/pytest#4774

Solution

Add requirements where we use pytest so we don't get incompatible versions of more-itertools

Result

pytest invocations continue to work on Python 2

[cosmicexplorer]

See d0432df and #7238 -- we are currently unable to bump the pytest version beyond 3.0.7 as a result of this error. We would like to bump the version to the one fixed by pytest-dev/pytest#4774, but we cannot at this exact moment go past pytest 3.7 due to the issue described in #6282 (see the code around the change in #7238).

We probably don't want to land this change as is, for the reasons described in pytest-dev/pytest#4770 (comment) (linked in #7238 -- we don't want to impose requirement constraints on pants users running pytest). It seems like this error would arise if you have overridden the pytest requirement in another repo's pants.ini or similar -- on the face of it it seems like you may be able to apply this change (pinning more-itertools) to that repo's requirements.txt so that the version of pytest you've overridden works with python 2.

However, I might be mistaken -- are you seeing the error described when developing in the pants repo, or is overriding the more-itertools requirement in your own repo causing an error?

[cheister]

I hadn't seen #7238. I was getting the error on pants 1.13.0.

The comment you linked (pytest-dev/pytest#4770) is referring to pinning specific versions of more_itertools in pytest. This change doesn't pin a specific version, it merely limits the version of more_itertools to something that works with whatever version of python you're running. Basically the same thing pytest itself is doing with pytest-dev/pytest#4774.

That seems more flexible than pinning pytest to 3.0.7 specifically.

[cosmicexplorer]

Ah, I misunderstood! Thank you for describing the difference -- this change sounds right enough to me then, although the specific implementation might be edited. @jsirois might the above^ be a reason to allow --pytest-requirements to be a list option, or would something like the current PR (hardcoding more-itertools<6.0.0 for py2, or maybe adding some more logic around it) be more appropriate?

[Eric-Arellano]

This looks right to me.

The key detail is python_version<3. Because more-itertools 6.0+ never will work with Python 2, users don't lose any compatibility here and I believe they can still specify a more specific version <6.0. If running with Python 3, there is no constraint at all. So, this change appears completely safe to make in that we won't be unreasonably constraining users at all.

Thank you for the patch!

--

Is this something we need to cherry-pick? cc @stuhood

[cosmicexplorer]

I think this is good to merge once both of the TODOs that @Eric-Arellano pointed out are added! Thanks again for the very specific and useful fix!

[stuhood]

Does this make it possible to revert (part of) #7238?

[Eric-Arellano]

@stuhood yes I believe we can completely revert #7238, and it would be good to do that as part of this PR. We shouldn’t care which Pytest people use so long as it’s <3.7. That PR was solely designed to fix this more-itertools issue, and this PR resolves the issue in a more flexible way.

[stuhood]

Thanks @cheister !