Skip to content
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

Replace setattr #493

Merged
merged 6 commits into from Jul 1, 2019
Merged

Replace setattr #493

merged 6 commits into from Jul 1, 2019

Conversation

tylerwince
Copy link
Contributor

setattr doesn't provide any value over setting the attribute directly via ___ = ___. Also cleaned up a call to isinstance where we were checking the same item twice in a single conditional.

@tylerwince
Copy link
Contributor Author

NOTE: This also contains the changes from #492 and should be merged after that one is accepted.

@ericwb ericwb added this to the Near Future milestone May 14, 2019
@tylerwince
Copy link
Contributor Author

@ericwb I resolved the merge conflicts after #492 was merged and this should be good to go as well

@ericwb
Copy link
Member

ericwb commented May 14, 2019

This PR can be merged after 1.6.1. I want 1.6.1 to contain just the regression bugs.

@tylerwince
Copy link
Contributor Author

tylerwince commented Jun 25, 2019

@ericwb now that 1.6.2 is released with bug fixes, can we get a review on this?

Copy link
Member

@ericwb ericwb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ericwb ericwb merged commit 45494c9 into PyCQA:master Jul 1, 2019
@@ -211,7 +211,7 @@ def linerange(node):
for key in strip.keys():
if hasattr(node, key):
strip[key] = getattr(node, key)
setattr(node, key, [])
node.key = []
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is bug.

>>> key = "foo"
>>> setattr(node, key, [])
>>> node.foo
[]
>>> node.key
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'Node' object has no attribute 'key'

@@ -222,7 +222,7 @@ def linerange(node):

for key in strip.keys():
if strip[key] is not None:
setattr(node, key, strip[key])
node.key = strip[key]
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here too

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants