Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

pep8 naming for Issue #44 #121

Closed
wants to merge 11 commits into
from

Conversation

Projects
None yet
3 participants

Hello,

here is a series of patches which bring checkers for naming conventions.
Its not fully complete, for example checks for class members are completely missing. Right now the following checks are there

  • Class names
  • Function names
  • Method names
  • Function argument names
  • Local variable names

I added some tests which are working under python2.7 and python3.2.

To make the tests possible I had to enhance the 'run_tests' method, because some syntax features of python3 are not available under python2. For example forcing keyword arguments.
My change checks the first three lines of the file to test. If this file contains a comment with 'python3 only' but pep8.py running under python2 the complete file is omitted. I added the same for 'python2 only'.

Feedback is very welcome

Contributor

florentx commented Dec 22, 2012

Thank you for this wonderful feature.
It might help when designing a new software to make it compliant with the conventions of PEP 8.

However, I don't plan to merge it in the next version. It is a little bit disruptive for existing libraries and it could give a lot of noise on large projects, while developers might have plenty of (bad) reasons to circumvent the naming conventions of PEP 8.
Moreover, it seems that the AST functions used in the patch are not compatible with Python 2.5.

I've written a short statement about the current choice (even if it might change some day):
https://pep8.readthedocs.org/en/latest/intro.html#disclaimer

I understand the points you made in the docs. Would it make sense, to enable this feature only if the user requested it. For example via a command line flag, disabled by default. So the users of the tool can choose if they use it or not.

Regarding py2.5, I will take a look on this.

Member

myint commented Dec 25, 2012

You may already know this, but pylint does something similar. pylint checks for conformance to a naming convention, which can be specified by the user.

@florentx florentx commented on the diff Dec 26, 2012

+ elif not self.GOOD_FUNCTION_NAME.match(node.name):
+ self.error_at_node(node, self.text)
+ elif not self.GOOD_FUNCTION_NAME.match(node.name):
+ self.error_at_node(node, self.text)
+
+
+class FunctionArgNamesASTCheck(BaseAstCheck):
+ """
+ The argument names of a function should be lowercase, with words separated
+ by underscores.
+
+ A classmethod should have 'cls' as first argument.
+ A method should have 'self' as first argument.
+ """
+
+ GOOD_ARG_NAME = re.compile('[a-z_][a-z0-9_]{0,30}$').match
@florentx

florentx Dec 26, 2012

Contributor

is the 31 chars limit in the PEP 8 recommendations?

No pep8 does not enforce the length.

I saw that you merged my pull request on a separate branch, where you did already a lot of changes (including the drop of the length check). I looked quickly over it and all the changes make sense to me. When I'm back from holidays, I will have a closer look at it. I will also use your branch for daily work so it sees some testing.

Is there anything else I can do right now to help you on this issue? Otherwise this pull request can be closed?

Contributor

florentx commented Feb 11, 2013

Please try flint and flint-naming.

@florentx florentx closed this Feb 11, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment