Remove redundant Python <= 2.6 code #270

hugovk · 2018-08-26T06:38:45Z

Support for Python 2.6 was officially removed in 9436948.

This removes some redundant code for Python 2.6 and earlier, and modernises some of the code using PyCharm's code inspections.

scoder

Thanks! Some of these files are copied from elsewhere and have been unmaintained for years, but overall, this PR cleans up quite a bit of corners.

scoder · 2018-08-26T06:42:45Z

benchmark/benchbase.py

@@ -11,7 +11,7 @@ def exec_(code, glob):
    if sys.version_info[0] >= 3:
        exec(code, glob)
    else:
-        exec("exec code in glob")
+        exec "exec code in glob"


exec() is a function in Py3, so this won't work.

This only gets called when sys.version_info[0] < 3, but shall I revert it anyway?

scoder · 2018-08-26T06:45:50Z

src/lxml/apihelpers.pxi

@@ -273,8 +273,7 @@ cdef _iter_attrib(attrib):
    # attrib will usually be a plain unordered dict
    if type(attrib) is dict:
        return sorted(attrib.items())
-    elif isinstance(attrib, _Attrib) or (
-            OrderedDict is not None and isinstance(attrib, OrderedDict)):
+    elif isinstance(attrib, _Attrib) or (isinstance(attrib, OrderedDict)):


This can be simplified to isinstance(attrib, (_Attrib, OrderedDict))

scoder · 2018-08-26T06:48:38Z

src/lxml/doctestcompare.py

-        diff_parts.append(self.collect_diff(want_doc, got_doc, html, 2))
+        diff_parts = ['Expected:', self.format_doc(want_doc, html, 2),
+                      'Got:', self.format_doc(got_doc, html, 2),
+                      'Diff:', self.collect_diff(want_doc, got_doc, html, 2)]


I didn't write this code, but I would guess that it was written this way to mimic the way the output is split into lines. I'd rather keep it in a line-by-line split.

scoder · 2018-08-26T06:49:01Z

src/lxml/etree.pyx

@@ -66,10 +66,7 @@ cdef object BytesIO, StringIO
 from io import BytesIO, StringIO

 cdef object OrderedDict = None


The = None is redundant now (and actually always was).

scoder · 2018-08-26T06:53:20Z

src/lxml/html/clean.py

+    if avoid_hosts is None:
+        avoid_hosts = _avoid_hosts
+    if avoid_classes is None:
+        avoid_classes = _avoid_classes


I'd rather keep all of these (in all functions you changed) as visible and introspectable default arguments. Make them tuples if you prefer (and if that doesn't break anything).

Sure, I'll just revert that commit.

scoder · 2018-08-26T07:07:40Z

src/lxml/tests/test_incremental_xmlfile.py

@@ -82,7 +82,7 @@ def test_write_Element_repeatedly(self):
        tree = self._parse_file()
        self.assertTrue(tree is not None)
        self.assertEqual(100, len(tree.getroot()))
-        self.assertEqual(set(['test']), set(el.tag for el in tree.getroot()))
+        self.assertEqual({'test'}, set(el.tag for el in tree.getroot()))


Use a set comprehension?

scoder · 2018-08-26T07:08:20Z

src/lxml/tests/test_incremental_xmlfile.py

-            "input", "keygen", "link", "meta", "param",
-            "source", "track", "wbr"
-        ])
+        void_elements = {"area", "base", "br", "col", "embed", "hr", "img",


I preferred the previous formatting (i.e. start a new line for the items).

scoder · 2018-08-26T07:10:04Z

src/lxml/tests/test_io.py

-            return getattr(self._tmpfile, name)
-else:
-    NamedTemporaryFile = tempfile.NamedTemporaryFile
+NamedTemporaryFile = tempfile.NamedTemporaryFile


This shouldn't be a global any more. Instead, the usages in the file should be prefixed with tempfile. to match the other usages of the module

scoder · 2018-08-26T07:11:30Z

src/lxml/tests/test_isoschematron.py


        schematron = isoschematron.Schematron(schema, store_report=False)
        self.assertTrue(schematron(tree_valid), schematron.error_log)
        valid = schematron(tree_invalid)
        self.assertTrue(not valid)
        self.assertTrue(schematron.validation_report is None,
            'validation reporting switched off, still: %s' %
-            (schematron.validation_report))
+                        schematron.validation_report)


Since the indentation is inconsistent between the two changes now, I'd prefer having both at the and of the previous line, right after the %.

scoder · 2018-08-26T07:13:15Z

tox.ini

@@ -4,7 +4,7 @@
 # and then run "tox" from this directory.

 [tox]
-envlist = py26, py27, py32, py33, py34
+envlist = py27, py33, py34


… and py35, py36, py37. Looks like this hasn't been used in a while. :)

This reverts commit 92faebc.

scoder · 2018-08-26T12:16:39Z

It obviously will not be called because it won't even pass through the Python 3 parser. ;-)

hugovk · 2018-08-26T13:39:19Z

Updated! I think I addressed everything, please let me know if I missed something.

scoder · 2018-08-26T13:49:56Z

Thanks!

hugovk · 2018-08-26T14:16:49Z

You're welcome, and thank you for lxml!

What do you think about dropping support for Python 3.3?

It's also EOL and little used. There probably wouldn't be much, if any, code to remove, but one benefit would be to drop a couple of builds from the CI, and one less thing to worry about.

Here's the pip installs for lxml from PyPI for July 2018:

python_version	percent	download_count
2.7	59.75%	637,606
3.6	22.04%	235,237
3.5	11.17%	119,194
3.4	4.08%	43,500
3.7	2.79%	29,729
2.6	0.10%	1,061
3.3	0.05%	553
3.8	0.01%	160
3.2	0.00%	41
None	0.00%	5
Total		1,067,086

Source: pypinfo --start-date 2018-07-01 --end-date 2018-07-31 --percent --markdown lxml pyversion

scoder · 2018-08-26T14:20:20Z

Sounds good to me.

hugovk · 2018-08-26T14:34:56Z

Please see PR #271.

hugovk added 9 commits August 25, 2018 20:53

Remove redundant code for Python <= 2.6

beaa4eb

Simplify Boolean expression

3c9475c

Compare None using 'is'/'is not' instead of equality operators

29b9f09

Replace mutable default argument

92faebc

Replace dictionary creation with dictionary literal

5703e6d

Replace function call with set literal

8e8fd0d

Replace list creation with list literal

2692f36

Remove redundant parentheses

5674dd2

Remove unnecessary backslash

1bb1c7e

scoder reviewed Aug 26, 2018

View reviewed changes

hugovk added 7 commits August 26, 2018 15:04

Simplify isinstance

0d146b0

Split lines for clarity

6359bb0

Remove redundant '= None'

38b89d1

Revert "Replace mutable default argument"

e3ab04c

This reverts commit 92faebc.

'assert False' more readable than 'assert 0'

37f87ef

Min version of LIBXML_VERSION is now 2.7

af50059

Use tempfile.NamedTemporaryFile directly

a6c7f49

hugovk added 7 commits August 26, 2018 15:22

Add newlines for dict's keys

22feab4

Use set comprehension

7b417ec

Start a new line for the items

6b8edfa

Keep on same line

dcdf7b7

Add newer Python versions

7063ee1

Keep parentheses

1e10b9d

Remove ununsed imports

9ac32de

scoder merged commit eb4d8c4 into lxml:master Aug 26, 2018

hugovk deleted the rm-2.6 branch August 26, 2018 14:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove redundant Python <= 2.6 code #270

Remove redundant Python <= 2.6 code #270

hugovk commented Aug 26, 2018

scoder left a comment

scoder Aug 26, 2018

hugovk Aug 26, 2018

scoder Aug 26, 2018

scoder Aug 26, 2018

scoder Aug 26, 2018

scoder Aug 26, 2018

hugovk Aug 26, 2018

scoder Aug 26, 2018

scoder Aug 26, 2018

scoder Aug 26, 2018

scoder Aug 26, 2018

scoder Aug 26, 2018

scoder commented Aug 26, 2018 via email

hugovk commented Aug 26, 2018

scoder commented Aug 26, 2018

hugovk commented Aug 26, 2018

scoder commented Aug 26, 2018

hugovk commented Aug 26, 2018

		@@ -66,10 +66,7 @@ cdef object BytesIO, StringIO
		from io import BytesIO, StringIO

		cdef object OrderedDict = None

Remove redundant Python <= 2.6 code #270

Remove redundant Python <= 2.6 code #270

Conversation

hugovk commented Aug 26, 2018

scoder 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

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

scoder commented Aug 26, 2018 via email

hugovk commented Aug 26, 2018

scoder commented Aug 26, 2018

hugovk commented Aug 26, 2018

scoder commented Aug 26, 2018

hugovk commented Aug 26, 2018