-
Notifications
You must be signed in to change notification settings - Fork 1.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
Drop Python 2 support #1918
Drop Python 2 support #1918
Changes from 11 commits
456086a
8decc12
7f2a926
7d6f32b
023ccbe
00d12b5
8637a20
b000d5b
173ee9e
97097fd
75f7921
ccaf7ca
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 |
---|---|---|
@@ -0,0 +1,20 @@ | ||
from pythonforandroid.recommendations import check_python_version | ||
from pythonforandroid.util import BuildInterruptingException, handle_build_exception | ||
|
||
|
||
def main(): | ||
""" | ||
Main entrypoint for running python-for-android as a script. | ||
""" | ||
|
||
try: | ||
# Check the Python version before importing anything heavier than | ||
# the util functions. This lets us provide a nice message about | ||
# incompatibility rather than having the interpreter crash if it | ||
# reaches unsupported syntax from a newer Python version. | ||
check_python_version() | ||
|
||
from pythonforandroid.toolchain import ToolchainCL | ||
ToolchainCL() | ||
except BuildInterruptingException as exc: | ||
handle_build_exception(exc) |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
data_files = [] | ||
|
||
|
||
|
||
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. nitpick: I hate spacing 😄 |
||
# must be a single statement since buildozer is currently parsing it, refs: | ||
# https://github.com/kivy/buildozer/issues/722 | ||
install_reqs = [ | ||
|
@@ -94,15 +95,15 @@ def recursively_include(results, directory, patterns): | |
install_requires=install_reqs, | ||
entry_points={ | ||
'console_scripts': [ | ||
'python-for-android = pythonforandroid.toolchain:main', | ||
'p4a = pythonforandroid.toolchain:main', | ||
'python-for-android = pythonforandroid.entrypoints:main', | ||
'p4a = pythonforandroid.entrypoints:main', | ||
], | ||
'distutils.commands': [ | ||
'apk = pythonforandroid.bdistapk:BdistAPK', | ||
], | ||
}, | ||
classifiers = [ | ||
'Development Status :: 4 - Beta', | ||
'Development Status :: 5 - Production/Stable', | ||
'Intended Audience :: Developers', | ||
'License :: OSI Approved :: MIT License', | ||
'Operating System :: Microsoft :: Windows', | ||
|
@@ -111,7 +112,6 @@ def recursively_include(results, directory, patterns): | |
'Operating System :: MacOS :: MacOS X', | ||
'Operating System :: Android', | ||
'Programming Language :: C', | ||
'Programming Language :: Python :: 2', | ||
'Programming Language :: Python :: 3', | ||
'Topic :: Software Development', | ||
'Topic :: Utilities', | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,6 +1,12 @@ | ||
|
||
import mock | ||
from mock import MagicMock | ||
# This test is still expected to support Python 2, as it tests | ||
# on-Android functionality that we still maintain | ||
try: # Python 3+ | ||
from unittest import mock | ||
from unittest.mock import MagicMock | ||
except ImportError: # Python 2 | ||
import mock | ||
from mock import MagicMock | ||
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. You guys are not lazy enough. 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. It isn't installed on my desktop :p I decided I preferred to be explicit about the Python 2 exception for the limited subset of files where we still need it. I'm willing to change it if you think it's better, but I consider this innocuous. 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. It doesn't need to be on your desktop since it's in the 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 would prefer keeping it in a way that works with the standard library ( 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. Thanks for sharing. I actually don't mind that much. I'm just being too lazy and I prefer one line over 4. Specially since it has to be repeated over a lot of (yet to come) tests files |
||
import os | ||
import shutil | ||
import sys | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
|
||
# This test is a special case that we expect to run under Python 2, so | ||
# include the necessary compatibility imports: | ||
try: # Python 3 | ||
from unittest import mock | ||
except ImportError: # Python 2 | ||
import mock | ||
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. Minor let's be lazy here too and just: import mock |
||
|
||
from pythonforandroid.recommendations import PY2_ERROR_TEXT | ||
from pythonforandroid import entrypoints | ||
|
||
|
||
def test_main_python2(): | ||
"""Test that running under Python 2 leads to the build failing, while | ||
running under a suitable version works fine. | ||
|
||
Note that this test must be run *using* Python 2 to truly test | ||
that p4a can reach the Python version check before importing some | ||
Python-3-only syntax and crashing. | ||
""" | ||
|
||
# Under Python 2, we should get a normal control flow exception | ||
# that is handled properly, not any other type of crash | ||
with mock.patch('sys.version_info') as fake_version_info, \ | ||
mock.patch('pythonforandroid.entrypoints.handle_build_exception') as handle_build_exception: | ||
|
||
fake_version_info.major = 2 | ||
fake_version_info.minor = 7 | ||
|
||
def check_python2_exception(exc): | ||
"""Check that the exception message is Python 2 specific.""" | ||
assert exc.message == PY2_ERROR_TEXT | ||
handle_build_exception.side_effect = check_python2_exception | ||
|
||
entrypoints.main() | ||
|
||
handle_build_exception.assert_called_once() |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,13 +9,19 @@ deps = | |
virtualenv | ||
py3: coveralls | ||
backports.tempfile | ||
# makes it possible to override pytest args, e.g. | ||
# tox -- tests/test_graph.py | ||
# posargs will be replaced by the tox args, so you can override pytest | ||
# args e.g. `tox -- tests/test_graph.py` | ||
commands = pytest {posargs:tests/} | ||
passenv = TRAVIS TRAVIS_* | ||
setenv = | ||
PYTHONPATH={toxinidir} | ||
|
||
[testenv:py27] | ||
# Note that the set of tests is not posargs-configurable here: we only | ||
# check a minimal set of Python 2 tests for the remaining Python 2 | ||
# functionality that we support | ||
commands = pytest tests/test_androidmodule_ctypes_finder.py tests/test_entrypoints_python2.py | ||
|
||
[testenv:py3] | ||
# for py3 env we will get code coverage | ||
commands = | ||
|
@@ -28,8 +34,13 @@ commands = flake8 pythonforandroid/ tests/ ci/ | |
|
||
[flake8] | ||
ignore = | ||
E123, E124, E126, | ||
E226, | ||
E402, E501, | ||
W503, | ||
W504 | ||
E123, # Closing bracket does not match indentation of opening bracket's line | ||
E124, # Closing bracket does not match visual indentation | ||
E126, # Continuation line over-indented for hanging indent | ||
E127, # Continuation line over-indented for visual indent | ||
E129, # Visually indented line with same indent as next logical line | ||
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. Nice that you're adding the description for them all 👏 |
||
E226, # Missing whitespace around arithmetic operator | ||
E402, # Module level import not at top of file | ||
E501, # Line too long (82 > 79 characters) | ||
W503, # Line break occurred before a binary operator | ||
W504 # Line break occurred after a binary operator |
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.
Nice to have comments about this. Best would be some unit tests. I can deal with this later