-
-
Notifications
You must be signed in to change notification settings - Fork 29.9k
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
Specify the minimum python version in the setup.py. #12144
Conversation
FWIW pip 9 has been around since late 2016: https://pip.pypa.io/en/stable/news/ |
The issue is not for how long something is around but is it shipped with Debian. Raspbian is a very popular OS platform among Home Assistant users. Around 30 % of the users are using Debian 8/ Rapbian 8 and there pip 1.5.x was shipped. pip 9.0.1 was introduced with Stretch and Raspian 9. |
Fair. I notice that the version of aiohttp that we depend upon also uses python_requires in its setup.py. https://github.com/aio-libs/aiohttp/blob/v2.3.7/setup.py#L113. I don't have an old version of pip to hand to test whether this attribute is simply ignored for old pips. |
setup.py
Outdated
@@ -75,6 +75,7 @@ | |||
zip_safe=False, | |||
platforms='any', | |||
install_requires=REQUIRES, | |||
python_requires=">=3.4", |
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.
'>=3.5.2'
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.
Excellent! Does #9328 need to be updated+closed then?
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.
You can update it
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.
I think aiohttp wants 3.5.3+ as minimum for the future.
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.
We are still Python 3.4 Pascal. We're waiting till aiohttp releases v3 to bump min requirement to 3.5
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.
Given this conversation, I don't think there is anything I need to do to get this PR ready.
Clearly it will be good to have the minimum python version explicitly stated in the codebase (setup.py).
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.
We already define the minimum required version in homeassistant.const
, can you import it from there? We also have a different minimum version between Windows and other OSes (because of the asyncio loop that we use).
Since we will soon align both min-versions, just sticking with 3.4.2 will do.
It looks that way. Installing aiohttp 2.3.10 with pip 8.0.0 doesn't show an error in regard of the presence of |
setup.py
Outdated
MIN_PY_VERSION = '.'.join(map(str, | ||
hass_const.REQUIRED_PYTHON_VER_WIN | ||
if sys.platform.startswith('win') | ||
else hass_const.REQUIRED_PYTHON_VER)) |
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.
continuation line under-indented for visual indent
setup.py
Outdated
@@ -61,9 +63,14 @@ | |||
'certifi>=2017.4.17', | |||
] | |||
|
|||
MIN_PY_VERSION = '.'.join(map(str, | |||
hass_const.REQUIRED_PYTHON_VER_WIN | |||
if sys.platform.startswith('win') |
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.
continuation line under-indented for visual indent
setup.py
Outdated
@@ -61,9 +63,14 @@ | |||
'certifi>=2017.4.17', | |||
] | |||
|
|||
MIN_PY_VERSION = '.'.join(map(str, | |||
hass_const.REQUIRED_PYTHON_VER_WIN |
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.
continuation line under-indented for visual indent
This added a little more complexity, but the current proposal now deals with both minimum versions. |
I knew it would get more complex, that's why I added this sentence at the bottom of my comment 😉
But this is even better. Awesome! |
Description:
Makes it explicit about which versions of python are supported. If
pip install homeassistant
is called from a version of python <= 3.4 the following error comes from pip (pip >= v9.0.1):`See also:
Checklist:
If user exposed functionality or configuration variables are added/changed:
If the code communicates with devices, web services, or third-party tools:
tox
run successfully. Your PR cannot be merged unless tests passREQUIREMENTS
variable (example).requirements_all.txt
by runningscript/gen_requirements_all.py
..coveragerc
.If the code does not interact with devices:
tox
run successfully. Your PR cannot be merged unless tests pass