From 9afb9ca5982bc1a04aa0d9641f8337286882bfb5 Mon Sep 17 00:00:00 2001 From: Nguyen Phuong An Date: Mon, 29 Aug 2016 18:15:30 +0700 Subject: [PATCH] Prevent use filter(lambda obj: test(obj), data) In Python3 [1], if we need filter on python3, replace filter(lambda obj: test(obj), data) with: [obj for obj in data if test(obj)]. This patch replaces filter function and introduces a hacking rule to prevent using filter in future. [1] https://wiki.openstack.org/wiki/Python3 Change-Id: I83d22108c02f8da007a7233e71a4a7fb833170ec --- HACKING.rst | 2 ++ neutron/hacking/checks.py | 13 +++++++++++++ neutron/objects/qos/policy.py | 4 ++-- neutron/policy.py | 2 +- neutron/tests/tempest/api/test_subnetpools.py | 10 ++++++---- neutron/tests/unit/hacking/test_checks.py | 7 +++++++ 6 files changed, 31 insertions(+), 7 deletions(-) diff --git a/HACKING.rst b/HACKING.rst index 347c0d4a590..5554ca92620 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -29,6 +29,8 @@ Neutron Specific Commandments - [N340] Check usage of .i18n (and neutron.i18n) - [N341] Check usage of _ from python builtins - [N342] String interpolation should be delayed at logging calls. +- [N344] Python 3: Do not use filter(lambda obj: test(obj), data). Replace it + with [obj for obj in data if test(obj)]. Creating Unit Tests ------------------- diff --git a/neutron/hacking/checks.py b/neutron/hacking/checks.py index 890df001c01..b09f56d3af0 100644 --- a/neutron/hacking/checks.py +++ b/neutron/hacking/checks.py @@ -75,6 +75,7 @@ log_warn = re.compile( r"(.)*LOG\.(warn)\(\s*('|\"|_)") unittest_imports_dot = re.compile(r"\bimport[\s]+unittest\b") unittest_imports_from = re.compile(r"\bfrom[\s]+unittest\b") +filter_match = re.compile(r".*filter\(lambda ") @flake8ext @@ -381,6 +382,17 @@ def check_delayed_string_interpolation(logical_line, filename, noqa): yield(0, msg) +@flake8ext +def check_python3_no_filter(logical_line): + """N344 - Use list comprehension instead of filter(lambda).""" + + msg = ("N343: Use list comprehension instead of " + "filter(lambda obj: test(obj), data) on python3.") + + if filter_match.match(logical_line): + yield(0, msg) + + def factory(register): register(validate_log_translations) register(use_jsonutils) @@ -400,3 +412,4 @@ def factory(register): register(check_builtins_gettext) register(check_unittest_imports) register(check_delayed_string_interpolation) + register(check_python3_no_filter) diff --git a/neutron/objects/qos/policy.py b/neutron/objects/qos/policy.py index c95f2bc3b62..73716b89789 100644 --- a/neutron/objects/qos/policy.py +++ b/neutron/objects/qos/policy.py @@ -212,8 +212,8 @@ class QosPolicy(rbac_db.NeutronRbacObject): def obj_make_compatible(self, primitive, target_version): def filter_rules(obj_names, rules): - return filter(lambda rule: - (rule['versioned_object.name'] in obj_names), rules) + return [rule for rule in rules if + rule['versioned_object.name'] in obj_names] _target_version = versionutils.convert_version_to_tuple(target_version) names = [] diff --git a/neutron/policy.py b/neutron/policy.py index adf7d68519f..19541a99218 100644 --- a/neutron/policy.py +++ b/neutron/policy.py @@ -112,7 +112,7 @@ def _build_subattr_match_rule(attr_name, attr, action, target): # typing for API attributes # Expect a dict as type descriptor validate = attr['validate'] - key = list(filter(lambda k: k.startswith('type:dict'), validate.keys())) + key = [k for k in validate.keys() if k.startswith('type:dict')] if not key: LOG.warning(_LW("Unable to find data type descriptor " "for attribute %s"), diff --git a/neutron/tests/tempest/api/test_subnetpools.py b/neutron/tests/tempest/api/test_subnetpools.py index 5bd222f8a72..a7a523eae94 100644 --- a/neutron/tests/tempest/api/test_subnetpools.py +++ b/neutron/tests/tempest/api/test_subnetpools.py @@ -110,13 +110,15 @@ class SubnetPoolsTest(SubnetPoolsTestBase): body = self._create_subnetpool(description='d1') self.assertEqual('d1', body['description']) sub_id = body['id'] - body = filter(lambda x: x['id'] == sub_id, - self.client.list_subnetpools()['subnetpools'])[0] + subnet_pools = [x for x in + self.client.list_subnetpools()['subnetpools'] if x['id'] == sub_id] + body = subnet_pools[0] self.assertEqual('d1', body['description']) body = self.client.update_subnetpool(sub_id, description='d2') self.assertEqual('d2', body['subnetpool']['description']) - body = filter(lambda x: x['id'] == sub_id, - self.client.list_subnetpools()['subnetpools'])[0] + subnet_pools = [x for x in + self.client.list_subnetpools()['subnetpools'] if x['id'] == sub_id] + body = subnet_pools[0] self.assertEqual('d2', body['description']) @test.idempotent_id('741d08c2-1e3f-42be-99c7-0ea93c5b728c') diff --git a/neutron/tests/unit/hacking/test_checks.py b/neutron/tests/unit/hacking/test_checks.py index 8ee3ecbd4d7..565e80afd06 100644 --- a/neutron/tests/unit/hacking/test_checks.py +++ b/neutron/tests/unit/hacking/test_checks.py @@ -339,6 +339,13 @@ class HackingTestCase(base.BaseTestCase): self.assertEqual( 1, len(list(checks.check_log_warn_deprecated(bad, 'f')))) + def test_check_python3_filter(self): + f = checks.check_python3_no_filter + self.assertLineFails(f, "filter(lambda obj: test(obj), data)") + self.assertLinePasses(f, "[obj for obj in data if test(obj)]") + self.assertLinePasses(f, "filter(function, range(0,10))") + self.assertLinePasses(f, "lambda x, y: x+y") + # The following is borrowed from hacking/tests/test_doctest.py. # Tests defined in docstring is easier to understand # in some cases, for example, hacking rules which take tokens as argument.