Ignore test files even if they are not in the top level

[cyli]

Currently, pep258 only ignores files if they are in the same directory in which pep257. This ignores all test files in subdirectories as well.

[keleshev]

the line:

is_script = source.startswith('#!') or filename.startswith('test_')

is kinda workaround. Nothing like that is ever said in PEP itself. But in reality PEP 257 does not apply well to test files. I will merge you patch, but maybe you have some deeper ideas on how this could be improved?

For example source.startswith('#!') is a total bullshit, since there are scripts that act as libraries at the same time. I was thinking of using this rule: only definitions exportable with star (from x import *) should be checked for PEP 257. This way by default definitions that start with underscore (like private functions) will not be checked -- good. Or you could specify __all__ and then only "public", "star-exportable" definitions will be checked. What do you think?

But then what's with tests? Ask people to write __all__ = [] in every test? No. Exclude definitions that start with test_? But then some BDD folks name their tests as should_.

[cyli]

@halst: Wow, thanks for responding so quickly!

I'm cool with not merging the PR if you are looking for a more robust method. I was going to add ignoring tests to my makefile command, but as I was looking over the pep257 code I noticed that line and I thought this PR would be more correct/more cooperative. :)

I wasn't sure about the source.startswith('#!') - I thought maybe some people would like to lint scripts, too.

With regards to the from x import *, It would make sense, when passing a module, to lint only all the public exportable definitions, in general.

I'm not sure how/whether you would like to limit the scope in which pep257 is used, but would it maybe make sense to let these things be command line (or configuration file) configuration options, if you can't cover all use cases?

pep8 supports an filename pattern matcher and an exclude option matcher, which would only exclude files, but as most people seem to put all their tests in one place or have the same naming convention, may allow people to configure as needed.

Maybe this could also be extrapolated to include module patterns, so users can specify to exclude _* as well?

[myint]

My preference would be to get rid all of this hard-coded stuff. The pep8-style --exclude option @cyli mentions seems like the way to go.

[keleshev]

Agree

[myint]

If this gets fixed, it would be good to get rid of the

================================================================================
Note: checks are relaxed for scripts (with #!) compared to modules.

thing too. That would make things nice and quiet.

[keleshev]

Yes

[keleshev]

What do you guys think of adding the following options:

--match=<pattern>     
          Check only files that exactly match <pattern> regular expression.
          Default is --match='(?!test_).*\.py' which matches files that 
          don't start with 'test_' but end with '.py'.
--match-dir=<pattern>
          Search only dirs that exactly match <pattern> regular expression.
          Default is --match-dir='[^\.].*', which matches all dirs that 
          don't start with a dot.

[cyli]

I think it'd definitely be an improvement over the hard coding and would give users a better way to customize, which is great!

I haven't seen too many command line tools let you match file names on a full regex (as opposed to a unix glob pattern), however, so my two concerns (which are probably not very important points) are:

1. compatibility with other command line tools
2. users possibly having to make the regex complex and hard to read if they want to exclude multiple prefixes

This is definitely nicer than before though :) Thank you for making this easier to use!

[keleshev]

Yeah, I have similar concerns.

I like unix globs better, but the problem with them is that you can't easily exclude things with them (no (?!something)).

Maybe the options should be --ignore='test_*' and --ignore-dir='.*'. But then it's necessary to enforce .py implicitly.

[treyhunner]

Why not allow an omit flag for excluding particular files or filename globs from being matched?

[jayvdb]

It would be good if pep257 supported the same command line (and config) options as pep8:

--exclude=patterns   exclude files or directories which match these comma
                     separated patterns (default:
                     .svn,CVS,.bzr,.hg,.git,__pycache__)

[keleshev]

@jayvdb Now pep257 has --match and --match-dir:

--match=<pattern>     check only files that exactly match <pattern> regular
                        expression; default is --match='(?!test_).*\.py' which
                        matches files that don't start with 'test_' but end
                        with '.py'
--match-dir=<pattern>
                        search only dirs that exactly match <pattern> regular
                        expression; default is --match-dir='[^\.].*', which
                        matches all dirs that don't start with a dot

Which are designed to cover all the discussed use-cases.

[jayvdb]

@keleshev , that isn't very nice if we want to exclude a single file, or a few dirs, and we need to maintain exclusion rules for pep8 & flake8.

e.g.
http://git.wikimedia.org/blob/pywikibot%2Fcore.git/b83e2e9e7dbf01800cc4b3bdef7f31d8ee775fdf/tox.ini#L139

Ideally, projects can add a pep257 block that is identical to pep8 and flake8 to their tox.ini. Having to maintain a second, more complex inverse, regex to do the same for pep257 alienates people who are not regex gurus.

[sigmavirus24]

@jayvdb you realize you don't need to bother if you just use flake8-docstrings, right?

[jayvdb]

@sigmavirus24 , sure am, and we are doing that (see linked tox.ini). We were doing that on patch upload, but that has now changed so that our linters can only run after patch approval (+1).

There are a lot less moving parts if we can run pep8 and pep257 directly, and from a security policy perspective there is less to audit if linters are run without a generic and extensible frameworks like flake8 & tox.

[sigmavirus24]

There are a lot less moving parts if we can run pep8 and pep257 directly,

Fair enough.

from a security policy perspective there is less to audit if linters are run without a generic and extensible frameworks like flake8 & tox.

This doesn't make much sense though and is probably better suited for discussion on code-quality@python.org or ##python-code-quality on Freenode.

[jayvdb]

Would you be happy with 'exclude' being added?

If yes, ... some details that are unimportant to me, but may as well decide before coding:
The same order of precedence as pep8 wrt --match/match-dir?
As a glob or a regex?

[samj1912]

This PR has not had any updates in 5 years. Closing this for now. If there's any interest in resuming this, feel free to reopen.