ross / requests-futures

Asynchronous Python HTTP Requests for Humans using Futures
Other
2.11k stars 152 forks source link

Use environment marker in install_requires #79

Closed mawbid closed 5 years ago

mawbid commented 5 years ago

Instead of adding a requirement on futures conditionally, always add it as a conditional requirement.

This is the recommended way, and enables installation with the Poetry tool.

Also give setuptools.setup a list instead of a tuple for the classifiers argument, since later versions complain about tuples.

benoit-pierre commented 5 years ago

If you're concerned about compatibility with older versions of setuptools, you could use the extras_require syntax instead (that's how install_requires requirements with environment markers are converted internally):

extras_require={ 
    ':python_version<"3.2"': ['futures>=2.1.3'],
}
ross commented 5 years ago

If you're concerned about compatibility with older versions of setuptools, you could use the extras_require syntax instead (that's how install_requires requirements with environment markers are converted internally):

Gave this a try and it half worked, but then ran into the fact that urllib3 uses more modern setuptools stuff that OSX has by default. Given that some of requests-futures's requirements are already requiring a newer version now it seems to make sense to give in and require it here too. Going to move forward with this PR as is. The error message seems like the best way to balance things.

benoit-pierre commented 5 years ago

What do you mean half-worked? It should be compatible with both older and newer versions of setuptools.

ross commented 5 years ago

What do you mean half-worked? It should be compatible with both older and newer versions of setuptools.

Yeah. That part worked and requests-futures was good, but new versions of urllib3 seem to be using environment makers which failed things further through the process. Thus just makes sense to ship this PR as it was.