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

Validate blank lines after a class definition in pandas #161

Open
datapythonista opened this issue Aug 29, 2019 · 4 comments
Open

Validate blank lines after a class definition in pandas #161

datapythonista opened this issue Aug 29, 2019 · 4 comments
Labels

Comments

@datapythonista
Copy link
Member

datapythonista commented Aug 29, 2019

The Python style guides don't have a standard for whether there should be a blank line after a class definition or not. So both of the next cases are valid:

class Foo:
    """
    Doc.
    """
    pass

class Bar:

    """
    Doc.
    """
    pass

But having both is a bit distracting when reading the code. In pandas-dev/pandas#28209 there has been work in making the first case our standard. But to make sure we don't reintroduce incorrect cases, we should validate in the CI that no cases exists. This can be done by checking in ci/code_checks.sh the regular expression mentioned in the description of the PR. In that file there are several other patterns that we want to avoid.

Before using the grep/regex is probably worth checking that flake8 is not able to check that (we ignore certain errors, may be we are ignoring that one in setup.cfg).

@Ayowolet
Copy link
Member

Ayowolet commented Sep 2, 2019

Can I attempt this? I will have loads of questions if that's okay.

@datapythonista
Copy link
Member Author

Sure @Ayowolet, thanks for the help. Let me know if you have any question.

@Ayowolet
Copy link
Member

Hi Marc,

so I thought to add this portion of code to the ci/code_checks.sh... does it look correct?

# Check for blank lines after the class definition
MSG='Check for extra blank lines after the class definition' ; echo $MSG
invgrep -R --include="*.rst" -E 'class.*:\n\n( )+"""' doc/source/
RET=$(($RET + $?)) ; echo $MSG "DONE"

@datapythonista
Copy link
Member Author

The include should be for python files, not rst. Other than that looks good. Feel free to open a PR even if you're unsure, not a big deal if there are errors. But it's easier to review, and also see in the CI if it's working as expected.

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

No branches or pull requests

2 participants