Drop support for EOL Python 3.3 #3060

hugovk · 2017-12-01T10:41:39Z

Continuation of #3045.

Tests are failing due to pytest (or rather py) dropping support for EOL Python 3.3 (pytest-dev/py#159 pytest-dev/py#165).

Here's the pip installs for pyinstaller from PyPI for the last month (via pypinfo --percent --pip pyinstaller pyversion), showing a very small fraction for 3.3:

python_version	percent	download_count
2.7	48.4%	26,288
3.6	24.9%	13,563
3.5	18.1%	9,857
3.4	7.1%	3,833
2.6	0.9%	496
3.7	0.5%	268
3.3	0.1%	50
3.2	0.0%	5
None	0.0%	2

This also cleans up some redundant code for Python <= 2.6.

htgoebel

Thanks for this pull-request. Beside some minore remarks (see inline notes), there are some major issues:

Files in PyInstaller/lib/modulegraph and tests/unit/test_modulegraph are incorporated from upstream and should not be changed. Otherwise applying upstream patches would become cumbersome.
All places where is_py34 is used need to be checked, too, and eventually changed into is_py3 or be removed. is_py34 basically means "is python 3, but not python3.3" and thus typically mark places where special treatment for Python 3.3 needs t be done.
Please group the changes into several commits with each commit including related changes. E.g.
- drop continuous integration tests for Python 3.3.
- "Remove xfail and skip for Python 3.3 (needs better wording
- requirements: Remove special treatment for python 3.3.
- remove compat flags for Python3.3 (need better wordking).

For the commit messages please also have a look at old messages. git gui blame and git log FILENAME are your friends.

Thanks.

htgoebel · 2017-12-01T11:11:12Z

.pylintrc

@@ -1,7 +1,7 @@
 [MASTER]
 # Add files or directories to the blacklist. They should be base names, not
 # paths.
-ignore=CVS,altgraph,macholib,pefile.py,six.py,unittest2,ordlookup,modulegraph
+ignore=CVS,altgraph,macholib,pefile.py,six.py,ordlookup,modulegraph


Nice, obviously it's time to clean this up, but then we clean up all outdated stuff: pefile.py and six.py are gone, too, CVS we never had.

And please put this into a separate commit, since it is not related to dropping support for any python version. Even unittest2 is not part of PyInstaller since long now.

htgoebel · 2017-12-01T11:11:43Z

PyInstaller/compat.py

@@ -28,7 +28,7 @@
 is_py3 = sys.version_info[0] == 3
 # Distinguish specific code for various Python versions.
 is_py27 = sys.version_info >= (2, 7) and sys.version_info < (3, 0)
-# PyInstaller supports only Python 3.3+
+# PyInstaller supports only Python 3.4+
 # Variables 'is_pyXY' mean that Python X.Y and up is supported.
 is_py34 = sys.version_info >= (3, 4)


This can be removed, too.

is_py34 is used here:

pyinstaller/PyInstaller/compat.py

Lines 172 to 180 in 8e199eb

# In Python 3.4 module 'imp' is deprecated and there is another way how

# to obtain magic value.

if is_py34:

import importlib.util

BYTECODE_MAGIC = importlib.util.MAGIC_NUMBER

else:

# This fallback should work with Python 2.7 and 3.3.

import imp

BYTECODE_MAGIC = imp.get_magic()

Should that be reversed into something like this?

# In Python 3.4 module 'imp' is deprecated and there is another way how # to obtain magic value. if is_py27: # This fallback should work with Python 2.7. import imp BYTECODE_MAGIC = imp.get_magic() else: # Python 3.4+ import importlib.util BYTECODE_MAGIC = importlib.util.MAGIC_NUMBER

And shall we delete is_py27 and replace it with is_py2?

Reversing is a good idea. I'd use is_py2, since it is clearer.

htgoebel · 2017-12-01T11:14:15Z

PyInstaller/lib/modulegraph/modulegraph.py

@@ -133,7 +133,7 @@
    "import sys,types,os; p = os.path.join(sys._getframe(1).f_locals['sitedir'], *('",
    "import sys,new,os; p = os.path.join(sys._getframe(1).f_locals['sitedir'], *('",
    "import sys, types, os;p = os.path.join(sys._getframe(1).f_locals['sitedir'], *('",
-    "import sys, types, os;pep420 = sys.version_info > (3, 3);p = os.path.join(sys._getframe(1).f_locals['sitedir'], *('",
+    "import sys, types, os;pep420 = sys.version_info > (3, 4);p = os.path.join(sys._getframe(1).f_locals['sitedir'], *('",


This must not be changed. These are pattern to match files generated by setuptools. And setuptools (or some version of it out in the wild) may still generate this code.

htgoebel · 2017-12-01T11:17:23Z

setup.py

@@ -158,6 +155,7 @@ def run(self):

 setup(
    install_requires=REQUIREMENTS,
+    python_requires='>=2.7, !=3.0.*, !=3.1.*, !=3.2.*, !=3.3.*',


Adding this keyword argument will requires a quite recent version of setuptools, which might not be a available on enterprise GNU/Linux distributions. Thus this should not be used.

If you have old setuptools, I believe this will be ignored, where it makes no difference.

If you have a recent setuptools, then this is useful and will help people install the correct version.

Shall I leave it or remove it?

I've left it in for now, can remove it if necessary.

I verified: Old setuptools will issue a warning, which is okay.

htgoebel · 2017-12-01T11:19:24Z

tests/requirements-libraries.txt

-# sphinx 1.5 requires Python 2.7 or >= 3.4
-sphinx==1.6.3; python_version != '3.3'
-sphinx==1.4.9; python_version == '3.3' # pyup: ignore
+sphinx==1.6.3



Please also remove the empty lines. There is no use in keeping them, now the "block" is gone. Same below.

htgoebel

Thanks for the quick update. There are still some left-overs of is_py34 in the code, e.g. a now unused import. Please remove it where no longer necessary.

The only place I'm spotting where is_py34 is used is tests/functional/test_multiprocess.py So I suggest using is_py3 there and drop is_py34 (this can go into one commit).

Regarding the commit messages: Please mind the subsystem prefixes, see https://pyinstaller.readthedocs.io/en/latest/development/commit-messages.html#commit-messages. Thanks.

htgoebel · 2017-12-01T13:48:59Z

PyInstaller/compat.py

@@ -28,7 +28,7 @@
 is_py3 = sys.version_info[0] == 3
 # Distinguish specific code for various Python versions.
 is_py27 = sys.version_info >= (2, 7) and sys.version_info < (3, 0)
-# PyInstaller supports only Python 3.3+
+# PyInstaller supports only Python 3.4+
 # Variables 'is_pyXY' mean that Python X.Y and up is supported.
 is_py34 = sys.version_info >= (3, 4)


Reversing is a good idea. I'd use is_py2, since it is clearer.

htgoebel · 2017-12-01T13:51:46Z

PyInstaller/loader/pyimod02_archive.py

-                # Python 3.3
-                self.pymagic = _frozen_importlib._MAGIC_BYTES
-            elif sys.version_info[1] == 4:
+            if sys.version_info[:2] == (3, 4):


Minor nitpick: Hmm, we could leave the test as version_info[1] == 4, since the version_info[0] is already tested above.

Should this run on Python 4.4, or only Python 3.4?

https://astrofrog.github.io/blog/2016/01/12/stop-writing-python-4-incompatible-code/

Since Python 3.4 is EOL 2019-03-16, and not even Python 4.0 is even planned. Thus I'm convinced this code will be dropped much earlier then 4.4 will appear :-)

htgoebel · 2017-12-01T13:56:37Z

tests/requirements-libraries.txt

@@ -73,12 +64,9 @@ ipython==5.4.1; python_version == '2.7' # pyup: ignore
 # dateutil.tz is a package in 2.5.0 and does not play nice with PyInstaller.
 python-dateutil==2.6.1

-pandas==0.20.3; python_version != '3.3'
-pandas==0.16.2; python_version == '3.3' # pyup: ignore
+pandas==0.20.3



Please remove the blank lines around these blocks, too.

htgoebel · 2017-12-08T14:17:46Z

By chance I found another place, where Py 3.3 code is used: See just below PY3_BASE_MODULES in compat.py. So I suggest greping the code for sys.version_info.

ghost · 2017-12-13T19:04:19Z

PyInstaller/compat.py

-
-
-if not is_py34:
-    class suppress(object):


This PR cannot be merged int it's current state because it removes this compatibility function. This should not be removed because it is required to run on Python 2.7.

Yes, that was a mistake in the rebase that needs sorting.

htgoebel · 2018-02-08T20:49:01Z

@hugovk Thanks again for taking the time for this pull-request. I had had planed to incorporate this for release 3.4 only, but now here are many test failing due to other packages dropping Pyhton 3.3. support. May I ask you to finish the clean up soon, so I can merge?! Thanks.

Cleanup to do:

resolve conflicts
fix what xoviat mentioned in Drop support for EOL Python 3.3 #3060 (comment)
251a75 should "fix up" the earlier change instead of being a commit on its own
60fe7ea should go into a pull-request of its own
Reword the commit messages: Please mind the subsystem prefixes, see the guideline.
Sort the commits: first test related, then "core", then cleanup, then docs. This is easy, once you have the subsystem prefix in the commit messages :-)

I can do this for you, but then this pull-request will be credited to you (github restriction).

ghost · 2018-02-08T20:54:02Z

@htgoebel I can take care of it if this will be merged quickly.

htgoebel · 2018-02-08T22:48:04Z

@xoviat Doing the quickly is the plan, but I wanted to credit to pull-request @hugovk. I suggest you take over if he does not respond until begin of next week. WDYT?

ghost · 2018-02-08T22:48:55Z

Sure. Fixing the CI is a priority though.

hugovk · 2018-02-09T03:59:02Z

It'd be great if you could finish this off, that'd be quickest. Sorry to leave this hanging for so long. I'm not too bothered about the credit. Thank you!

ghost · 2018-02-14T23:12:11Z

cc @htgoebel. See gh-3288.

htgoebel · 2018-02-17T20:49:51Z

Closed in favour of #3288.

htgoebel suggested changes Dec 1, 2017

View reviewed changes

htgoebel changed the title ~~Drop support for EOL Python <=2.6 and 3.3~~ Drop support for EOL Python 3.3 Dec 1, 2017

htgoebel suggested changes Dec 1, 2017

View reviewed changes

htgoebel added this to the PyInstaller 3.4 milestone Dec 7, 2017

hugovk added 11 commits December 8, 2017 16:22

Drop continuous integration tests for Python 3.3

489b5f1

Remove xfail for Python 3.3 and update skipif for Python 3

73c7c1f

Requirements: Remove special treatment for Python 3.3

177a371

Fail hard on Python 3.3

09f4b54

Update docs to remove Python 3.3

1dce147

Remove special treatment for Python 3.3

3b47e3c

Clean up outdated from blacklist

60fe7ea

Specify Python versions this distribution is compatible with

a5653f6

Reduce pair of nested tests to a single one

460192a

Revert to version_info[1] == 4

251a759

Replace is_py34 with is_py3

b1815d1

ghost reviewed Dec 13, 2017

View reviewed changes

ghost mentioned this pull request Feb 4, 2018

Linux usb hook fixes #3269

Merged

ghost mentioned this pull request Feb 14, 2018

Drop 3.3 support #3288

Closed

htgoebel added the solution:duplicate Resolved: This issue is a duplicate. label Feb 17, 2018

htgoebel closed this Feb 17, 2018

hugovk deleted the rm-3.3 branch March 27, 2018 07:35

dchevell mentioned this pull request Jul 9, 2018

Deprecate werkzeug.contrib.cache pallets-eco/flask-caching#75

Merged

github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Drop support for EOL Python 3.3 #3060

Drop support for EOL Python 3.3 #3060

hugovk commented Dec 1, 2017 •

edited

Loading

htgoebel left a comment •

edited

Loading

htgoebel Dec 1, 2017

htgoebel Dec 1, 2017

hugovk Dec 1, 2017

htgoebel Dec 1, 2017

htgoebel Dec 1, 2017

htgoebel Dec 1, 2017

hugovk Dec 1, 2017

hugovk Dec 1, 2017

htgoebel Dec 1, 2017

htgoebel Dec 1, 2017

htgoebel left a comment

htgoebel Dec 1, 2017

htgoebel Dec 1, 2017

hugovk Dec 1, 2017

htgoebel Dec 2, 2017

htgoebel Dec 1, 2017

htgoebel commented Dec 8, 2017

ghost Dec 13, 2017

hugovk Dec 13, 2017

htgoebel commented Feb 8, 2018

ghost commented Feb 8, 2018

htgoebel commented Feb 8, 2018

ghost commented Feb 8, 2018

hugovk commented Feb 9, 2018

ghost commented Feb 14, 2018

htgoebel commented Feb 17, 2018

	# In Python 3.4 module 'imp' is deprecated and there is another way how
	# to obtain magic value.
	if is_py34:
	import importlib.util
	BYTECODE_MAGIC = importlib.util.MAGIC_NUMBER
	else:
	# This fallback should work with Python 2.7 and 3.3.
	import imp
	BYTECODE_MAGIC = imp.get_magic()

Drop support for EOL Python 3.3 #3060

Drop support for EOL Python 3.3 #3060

Conversation

hugovk commented Dec 1, 2017 • edited Loading

htgoebel left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

htgoebel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

htgoebel commented Dec 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

htgoebel commented Feb 8, 2018

ghost commented Feb 8, 2018

htgoebel commented Feb 8, 2018

ghost commented Feb 8, 2018

hugovk commented Feb 9, 2018

ghost commented Feb 14, 2018

htgoebel commented Feb 17, 2018

hugovk commented Dec 1, 2017 •

edited

Loading

htgoebel left a comment •

edited

Loading