Merge ~cjwatson/launchpad:bugbear-getattr-setattr into launchpad:master

Proposed by Colin Watson
Status: Merged
Approved by: Colin Watson
Approved revision: 39015f0c2ec9c6f2bf3fe940bca22d605c566814
Merge reported by: Otto Co-Pilot
Merged at revision: not available
Proposed branch: ~cjwatson/launchpad:bugbear-getattr-setattr
Merge into: launchpad:master
Diff against target: 241 lines (+23/-34)
9 files modified
lib/lp/answers/tests/test_question_workflow.py (+10/-14)
lib/lp/blueprints/model/sprint.py (+1/-1)
lib/lp/code/model/branchtarget.py (+2/-8)
lib/lp/codehosting/vfs/branchfs.py (+1/-1)
lib/lp/codehosting/vfs/transport.py (+1/-1)
lib/lp/registry/tests/test_product.py (+4/-4)
lib/lp/services/webapp/error.py (+1/-1)
lib/lp/services/webapp/publisher.py (+1/-1)
lib/lp/testing/factory.py (+2/-3)
Reviewer Review Type Date Requested Status
Jürgen Gmach Approve
Review via email: mp+454020@code.launchpad.net

Commit message

Remove useless uses of getattr/setattr

Description of the change

`flake8-bugbear` points out:

  B009: Do not call getattr(x, 'attr'), instead use normal property access: x.attr. Missing a default to getattr will cause an AttributeError to be raised for non-existent properties. There is no additional safety in using getattr if you know the attribute name ahead of time.

  B010: Do not call setattr(x, 'attr', val), instead use normal property access: x.attr = val. There is no additional safety in using setattr if you know the attribute name ahead of time.

To post a comment you must log in.
Revision history for this message
Jürgen Gmach (jugmac00) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
diff --git a/lib/lp/answers/tests/test_question_workflow.py b/lib/lp/answers/tests/test_question_workflow.py
index c0e61ce..2569042 100644
--- a/lib/lp/answers/tests/test_question_workflow.py
+++ b/lib/lp/answers/tests/test_question_workflow.py
@@ -426,7 +426,7 @@ class RequestInfoTestCase(BaseAnswerTrackerWorkflowTestCase):
426 self.assertRaises(Unauthorized, getattr, self.question, "requestInfo")426 self.assertRaises(Unauthorized, getattr, self.question, "requestInfo")
427427
428 login_person(self.answerer)428 login_person(self.answerer)
429 getattr(self.question, "requestInfo")429 self.question.requestInfo
430430
431431
432class GiveInfoTestCase(BaseAnswerTrackerWorkflowTestCase):432class GiveInfoTestCase(BaseAnswerTrackerWorkflowTestCase):
@@ -473,7 +473,7 @@ class GiveInfoTestCase(BaseAnswerTrackerWorkflowTestCase):
473 self.assertRaises(Unauthorized, getattr, self.question, "giveInfo")473 self.assertRaises(Unauthorized, getattr, self.question, "giveInfo")
474474
475 login_person(self.owner)475 login_person(self.owner)
476 getattr(self.question, "giveInfo")476 self.question.giveInfo
477477
478478
479class GiveAnswerTestCase(BaseAnswerTrackerWorkflowTestCase):479class GiveAnswerTestCase(BaseAnswerTrackerWorkflowTestCase):
@@ -588,7 +588,7 @@ class GiveAnswerTestCase(BaseAnswerTrackerWorkflowTestCase):
588 self.assertRaises(Unauthorized, getattr, self.question, "giveAnswer")588 self.assertRaises(Unauthorized, getattr, self.question, "giveAnswer")
589589
590 login_person(self.answerer)590 login_person(self.answerer)
591 getattr(self.question, "giveAnswer")591 self.question.giveAnswer
592592
593593
594class LinkFAQTestCase(BaseAnswerTrackerWorkflowTestCase):594class LinkFAQTestCase(BaseAnswerTrackerWorkflowTestCase):
@@ -680,7 +680,7 @@ class LinkFAQTestCase(BaseAnswerTrackerWorkflowTestCase):
680 self.assertRaises(Unauthorized, getattr, self.question, "linkFAQ")680 self.assertRaises(Unauthorized, getattr, self.question, "linkFAQ")
681681
682 login_person(self.answerer)682 login_person(self.answerer)
683 getattr(self.question, "linkFAQ")683 self.question.linkFAQ
684684
685685
686class ConfirmAnswerTestCase(BaseAnswerTrackerWorkflowTestCase):686class ConfirmAnswerTestCase(BaseAnswerTrackerWorkflowTestCase):
@@ -856,7 +856,7 @@ class ConfirmAnswerTestCase(BaseAnswerTrackerWorkflowTestCase):
856 )856 )
857857
858 login_person(self.owner)858 login_person(self.owner)
859 getattr(self.question, "confirmAnswer")859 self.question.confirmAnswer
860860
861861
862class ReopenTestCase(BaseAnswerTrackerWorkflowTestCase):862class ReopenTestCase(BaseAnswerTrackerWorkflowTestCase):
@@ -971,7 +971,7 @@ class ReopenTestCase(BaseAnswerTrackerWorkflowTestCase):
971 self.assertRaises(Unauthorized, getattr, self.question, "reopen")971 self.assertRaises(Unauthorized, getattr, self.question, "reopen")
972972
973 login_person(self.owner)973 login_person(self.owner)
974 getattr(self.question, "reopen")974 self.question.reopen
975975
976976
977class ExpireQuestionTestCase(BaseAnswerTrackerWorkflowTestCase):977class ExpireQuestionTestCase(BaseAnswerTrackerWorkflowTestCase):
@@ -1014,7 +1014,7 @@ class ExpireQuestionTestCase(BaseAnswerTrackerWorkflowTestCase):
1014 )1014 )
10151015
1016 login_person(self.answerer)1016 login_person(self.answerer)
1017 getattr(self.question, "expireQuestion")1017 self.question.expireQuestion
10181018
10191019
1020class RejectTestCase(BaseAnswerTrackerWorkflowTestCase):1020class RejectTestCase(BaseAnswerTrackerWorkflowTestCase):
@@ -1103,13 +1103,10 @@ class RejectTestCase(BaseAnswerTrackerWorkflowTestCase):
1103 # clear authorization cache for check_permission1103 # clear authorization cache for check_permission
1104 clear_cache()1104 clear_cache()
1105 self.assertTrue(1105 self.assertTrue(
1106 getattr(self.question, "reject"),1106 self.question.reject, "Answer contact cannot reject question."
1107 "Answer contact cannot reject question.",
1108 )1107 )
1109 login_person(self.admin)1108 login_person(self.admin)
1110 self.assertTrue(1109 self.assertTrue(self.question.reject, "Admin cannot reject question.")
1111 getattr(self.question, "reject"), "Admin cannot reject question."
1112 )
11131110
1114 def testRejectPermission_indirect_answer_contact(self):1111 def testRejectPermission_indirect_answer_contact(self):
1115 # Indirect answer contacts (for a distribution) can reject1112 # Indirect answer contacts (for a distribution) can reject
@@ -1121,6 +1118,5 @@ class RejectTestCase(BaseAnswerTrackerWorkflowTestCase):
1121 self.answerer.addLanguage(getUtility(ILanguageSet)["en"])1118 self.answerer.addLanguage(getUtility(ILanguageSet)["en"])
1122 self.ubuntu.addAnswerContact(self.answerer, self.answerer)1119 self.ubuntu.addAnswerContact(self.answerer, self.answerer)
1123 self.assertTrue(1120 self.assertTrue(
1124 getattr(self.question, "reject"),1121 self.question.reject, "Answer contact cannot reject question."
1125 "Answer contact cannot reject question.",
1126 )1122 )
diff --git a/lib/lp/blueprints/model/sprint.py b/lib/lp/blueprints/model/sprint.py
index 15f20ea..7b5fc16 100644
--- a/lib/lp/blueprints/model/sprint.py
+++ b/lib/lp/blueprints/model/sprint.py
@@ -425,7 +425,7 @@ class HasSprintsMixin:
425425
426 Subclasses must overwrite this method if it doesn't suit them.426 Subclasses must overwrite this method if it doesn't suit them.
427 """427 """
428 table = getattr(self, "__storm_table__")428 table = self.__storm_table__
429 return [429 return [
430 getattr(Specification, table.lower()) == self,430 getattr(Specification, table.lower()) == self,
431 Specification.id == SprintSpecification.specification_id,431 Specification.id == SprintSpecification.specification_id,
diff --git a/lib/lp/code/model/branchtarget.py b/lib/lp/code/model/branchtarget.py
index 998c391..ceb60fa 100644
--- a/lib/lp/code/model/branchtarget.py
+++ b/lib/lp/code/model/branchtarget.py
@@ -184,10 +184,7 @@ class PackageBranchTarget(_BaseBranchTarget):
184184
185 result = sorted_version_numbers(185 result = sorted_version_numbers(
186 result,186 result,
187 key=lambda branch_info: (187 key=lambda branch_info: (branch_info[1].name, branch_info[0].id),
188 getattr(branch_info[1], "name"),
189 branch_info[0].id,
190 ),
191 )188 )
192189
193 if limit_results is not None:190 if limit_results is not None:
@@ -438,10 +435,7 @@ class ProductBranchTarget(_BaseBranchTarget):
438435
439 result = sorted_version_numbers(436 result = sorted_version_numbers(
440 result,437 result,
441 key=lambda branch_info: (438 key=lambda branch_info: (branch_info[1].name, branch_info[0].id),
442 getattr(branch_info[1], "name"),
443 branch_info[0].id,
444 ),
445 )439 )
446440
447 if limit_results is not None:441 if limit_results is not None:
diff --git a/lib/lp/codehosting/vfs/branchfs.py b/lib/lp/codehosting/vfs/branchfs.py
index 74461c9..3010e69 100644
--- a/lib/lp/codehosting/vfs/branchfs.py
+++ b/lib/lp/codehosting/vfs/branchfs.py
@@ -502,7 +502,7 @@ class AsyncLaunchpadTransport(AsyncVirtualTransport):
502 @no_traceback_failures502 @no_traceback_failures
503 def real_mkdir(result):503 def real_mkdir(result):
504 transport, path = result504 transport, path = result
505 return getattr(transport, "mkdir")(path, mode)505 return transport.mkdir(path, mode)
506506
507 deferred.addCallback(real_mkdir)507 deferred.addCallback(real_mkdir)
508 deferred.addErrback(maybe_make_branch_in_db)508 deferred.addErrback(maybe_make_branch_in_db)
diff --git a/lib/lp/codehosting/vfs/transport.py b/lib/lp/codehosting/vfs/transport.py
index bcdb9d3..b896c65 100644
--- a/lib/lp/codehosting/vfs/transport.py
+++ b/lib/lp/codehosting/vfs/transport.py
@@ -247,7 +247,7 @@ class AsyncVirtualTransport(Transport):
247 "cannot move between underlying transports"247 "cannot move between underlying transports"
248 )248 )
249 )249 )
250 return getattr(from_transport, "rename")(from_path, to_path)250 return from_transport.rename(from_path, to_path)
251251
252 deferred.addCallback(check_transports_and_rename)252 deferred.addCallback(check_transports_and_rename)
253 return deferred253 return deferred
diff --git a/lib/lp/registry/tests/test_product.py b/lib/lp/registry/tests/test_product.py
index 4b22c33..1ef5135 100644
--- a/lib/lp/registry/tests/test_product.py
+++ b/lib/lp/registry/tests/test_product.py
@@ -1443,9 +1443,9 @@ class TestProduct(TestCaseWithFactory):
1443 )1443 )
1444 ordinary_user = self.factory.makePerson()1444 ordinary_user = self.factory.makePerson()
1445 with person_logged_in(ordinary_user):1445 with person_logged_in(ordinary_user):
1446 setattr(product, "date_next_suggest_packaging", "foo")1446 product.date_next_suggest_packaging = "foo"
1447 with person_logged_in(product.owner):1447 with person_logged_in(product.owner):
1448 setattr(product, "date_next_suggest_packaging", "foo")1448 product.date_next_suggest_packaging = "foo"
14491449
1450 def test_set_launchpad_AnyAllowedPerson_proprietary_product(self):1450 def test_set_launchpad_AnyAllowedPerson_proprietary_product(self):
1451 # Only people with grants for a private product can set1451 # Only people with grants for a private product can set
@@ -1473,7 +1473,7 @@ class TestProduct(TestCaseWithFactory):
1473 "foo",1473 "foo",
1474 )1474 )
1475 with person_logged_in(owner):1475 with person_logged_in(owner):
1476 setattr(product, "date_next_suggest_packaging", "foo")1476 product.date_next_suggest_packaging = "foo"
1477 # A user with a policy grant for the product can access attributes1477 # A user with a policy grant for the product can access attributes
1478 # of a private product.1478 # of a private product.
1479 with person_logged_in(owner):1479 with person_logged_in(owner):
@@ -1484,7 +1484,7 @@ class TestProduct(TestCaseWithFactory):
1484 {InformationType.PROPRIETARY: SharingPermission.ALL},1484 {InformationType.PROPRIETARY: SharingPermission.ALL},
1485 )1485 )
1486 with person_logged_in(ordinary_user):1486 with person_logged_in(ordinary_user):
1487 setattr(product, "date_next_suggest_packaging", "foo")1487 product.date_next_suggest_packaging = "foo"
14881488
1489 def test_userCanView_caches_known_users(self):1489 def test_userCanView_caches_known_users(self):
1490 # userCanView() maintains a cache of users known to have the1490 # userCanView() maintains a cache of users known to have the
diff --git a/lib/lp/services/webapp/error.py b/lib/lp/services/webapp/error.py
index 4c451f9..b0f7f3f 100644
--- a/lib/lp/services/webapp/error.py
+++ b/lib/lp/services/webapp/error.py
@@ -67,7 +67,7 @@ class SystemErrorView(LaunchpadView):
67 self.request.response.removeAllNotifications()67 self.request.response.removeAllNotifications()
68 if self.response_code is not None:68 if self.response_code is not None:
69 self.request.response.setStatus(self.response_code)69 self.request.response.setStatus(self.response_code)
70 if getattr(self.request, "oopsid") is not None:70 if self.request.oopsid is not None:
71 self.request.response.addHeader(71 self.request.response.addHeader(
72 "X-Lazr-OopsId", self.request.oopsid72 "X-Lazr-OopsId", self.request.oopsid
73 )73 )
diff --git a/lib/lp/services/webapp/publisher.py b/lib/lp/services/webapp/publisher.py
index 4702b67..bf64b0d 100644
--- a/lib/lp/services/webapp/publisher.py
+++ b/lib/lp/services/webapp/publisher.py
@@ -185,7 +185,7 @@ class redirection:
185 redirections = getattr(fn, "__redirections__", None)185 redirections = getattr(fn, "__redirections__", None)
186 if redirections is None:186 if redirections is None:
187 redirections = {}187 redirections = {}
188 setattr(fn, "__redirections__", redirections)188 fn.__redirections__ = redirections
189 redirections[self.name] = self.status189 redirections[self.name] = self.status
190 return fn190 return fn
191191
diff --git a/lib/lp/testing/factory.py b/lib/lp/testing/factory.py
index 7605ed7..71fac63 100644
--- a/lib/lp/testing/factory.py
+++ b/lib/lp/testing/factory.py
@@ -1552,8 +1552,7 @@ class LaunchpadObjectFactory(ObjectFactory):
15521552
1553 # Sort them1553 # Sort them
1554 related_series_branch_info = sorted_version_numbers(1554 related_series_branch_info = sorted_version_numbers(
1555 series_branch_info,1555 series_branch_info, key=lambda branch_info: branch_info[1].name
1556 key=lambda branch_info: (getattr(branch_info[1], "name")),
1557 )1556 )
15581557
1559 # Add a development branch at the start of the list.1558 # Add a development branch at the start of the list.
@@ -1641,7 +1640,7 @@ class LaunchpadObjectFactory(ObjectFactory):
16411640
1642 related_package_branch_info = sorted_version_numbers(1641 related_package_branch_info = sorted_version_numbers(
1643 related_package_branch_info,1642 related_package_branch_info,
1644 key=lambda branch_info: (getattr(branch_info[1], "name")),1643 key=lambda branch_info: branch_info[1].name,
1645 )1644 )
16461645
1647 return (1646 return (

Subscribers

People subscribed via source and target branches

to status/vote changes: