-
-
Notifications
You must be signed in to change notification settings - Fork 91
exclude legacy dependencies when using python3 #85
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,7 +45,6 @@ | |
), | ||
python_requires='>=2.7,!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*', | ||
install_requires=[ | ||
'backports.functools_lru_cache', | ||
'six>=1.11.0', | ||
'more_itertools>=2.6', | ||
], | ||
|
@@ -72,6 +71,13 @@ | |
'codecov', | ||
|
||
'pytest-cov', | ||
], | ||
':python_version<"3.3"': [ | ||
# functools_lru_cache has been added to stdlib in Python 3.2 | ||
# however it's only got support for ``typed`` arg in Python 3.3 | ||
'backports.functools_lru_cache', | ||
], | ||
'testing:python_version<"3.3"': [ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've rejected this change before, because backports.unittest_mock is intended to be unconditionally included and it provides the conditionality (only including mock on the platforms where needed). In addition to making these dependency declarations more complicated in this project, it also implies that this is the rule that should be copied into every project that uses it. I reject that notion in that it violates DRY and adds to the cognitive burden of every project that uses it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are you saying that we should revert it? |
||
'backports.unittest_mock', | ||
], | ||
}, | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This introduction of ranged comparators in extras will create a (possibly backward-incompatible) expectation on the project to require Setuptools 17.1 or later. That's probably acceptable at this stage, but it should probably be declared in the changelog, or even better in a pyproject.toml. Personally, I'd like for these backports modules to be simply declared/included unconditionally.
The time to download a tiny module like this is small and it's mostly machine time, whereas the added complexity of the package declaration and the additional cognitive burden of that complexity consumes real human work.
I think I'd prefer the simpler declaration that avoids the need for environment markers and comments, even if it doesn't precisely define where the package is absolutely needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jaraco We install
setuptools>=28.2
in Travis: https://github.com/cherrypy/cheroot/blob/6821264/.travis.yml#L156Also Python 2.7.14 is shipped with
setuptools==28.8.0
: https://github.com/python/cpython/tree/v2.7.14/Lib/ensurepip/_bundled(next patch release of 2.7.x should have https://github.com/python/cpython/tree/2.7/Lib/ensurepip/_bundled
>=39.0.1
)So it looks okay to already use these features. (of course for envs where OS package manager controls things it might differ)