Skip to content

Fix #28 - #219

Closed
attilaolah wants to merge 2 commits into
PyCQA:masterfrom
attilaolah:master
Closed

Fix #28#219
attilaolah wants to merge 2 commits into
PyCQA:masterfrom
attilaolah:master

Conversation

@attilaolah

Copy link
Copy Markdown

This is a simple fix for #28. It also enables the test added in 82c3a9e.

Comment thread pep8.py

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I wonder though if this is correct, and how should we handle classes defined inside functions that have methods that are not separated by a blank line.

@chrismedrela

Copy link
Copy Markdown

Well, nested_function doesn't say if we are inside a nested function. It indicates if there was any function definition before. Consider this example:

def blb():
    pass


class Foo(object):
    def bar(self):
        pass
    def foo(self): # no blank line before
        pass

This should result in an E302 error, but your patch hides it since it assumes that foo is nested inside blb.

@attilaolah

Copy link
Copy Markdown
Author

Good catch. Let me revisit my fork :)

@gward

gward commented Nov 24, 2013

Copy link
Copy Markdown
Contributor

Pretty sure PR #232 is a better approach. Maybe close this PR?

@attilaolah

Copy link
Copy Markdown
Author

Yes, let's close this in favour of #232.

@attilaolah attilaolah closed this Nov 25, 2013
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.

3 participants