-
Notifications
You must be signed in to change notification settings - Fork 188
Ignore test files even if they are not in the top level #22
Conversation
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 But then what's with tests? Ask people to write |
@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 With regards to the 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 |
My preference would be to get rid all of this hard-coded stuff. The |
Agree |
If this gets fixed, it would be good to get rid of the
thing too. That would make things nice and quiet. |
Yes |
What do you guys think of adding the following options:
|
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:
This is definitely nicer than before though :) Thank you for making this easier to use! |
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 Maybe the options should be |
Why not allow an |
It would be good if pep257 supported the same command line (and config) options as pep8:
|
@jayvdb Now
Which are designed to cover all the discussed use-cases. |
@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. 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. |
@jayvdb you realize you don't need to bother if you just use flake8-docstrings, right? |
@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. |
Fair enough.
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. |
Would you be happy with 'exclude' being added? If yes, ... some details that are unimportant to me, but may as well decide before coding: |
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. |
Currently, pep258 only ignores files if they are in the same directory in which pep257. This ignores all test files in subdirectories as well.