Files passed as arguments ignored by --match

[jpulec]

It looks like when you pass specific filenames to pep257, it ignores if you have set the --match option.

For example:

pep257 foo.py bar.py --match='(?!foo).*\.py'

Will still check foo.py and bar.py. While I understand why this makes sense, since there is a default match pattern that ignores 'test_.', and you want to be able to easily use pep257 on a file prefixed with 'test._' if desired, iit means that pep257 doesn't perform as one might expect when using something like xargs to pass a bunch of files into it.

[jpulec]

PR in #112

[jayvdb]

Close?

[jpulec]

Is this seen as a non-standard use case, or that it breaks expected behavior?

[Nurdok]

I'm not sure how I feel about this. On the one hand, I get your point, but I also feel that passing an explicit file name should take precedence over the matching regex. Anyone else cares to pitch in?

[erichiller]

Was this reverted? The behavior I am seeing shows that an explicitly passed in file will override match

[xmnlab]

AFAIK if it overrides match it will generate conflicts when using git pre-commit hook.

[aaaaahaaaaa]

I'm noticing it indeed overrides match and match_dir configs when using the pre-commit hook since the files are passed as positional arguments. Which makes the hook useless as far as I can tell because it checks files which are meant to be ignored.

A workaround is to use a local hook to run the shell command directly, which allows to use your project configuration in a consistent way:

- repo: local
  hooks:
    - id: pydocstyle
      name: pydocstyle
      types: [python]
      entry: pydocstyle
      language: system
      pass_filenames: false

[danmoz]

On the one hand, I get your point, but I also feel that passing an explicit file name should take precedence over the matching regex. Anyone else cares to pitch in?

Currently the only way to exclude certain files is using negative lookahead patterns like --match='(?!test_).*\.py'. Since this is ignored when files are passed in (i.e. when using pre-commit) there is no way to exclude files passed in on the CLI.

If you're using pydocstyles pre-commit support, you'll be passing in the list of changed files every time you do a git commit, but you will still want to exclude certain files, which this issue prevents. The only thing pre-commit users can do is use pass_filenames: false (as suggested in previous comment) which causes pydocstyle to run on the whole codebase. But in a large codebase, this is too slow.

IMHO, for the pre-commit support to be useful, there needs to be a way to exclude certain file patterns, even if they are passed in explicitly on the CLI.

What if we introduced a switch that prevents match from being ignored when filenames are passed in? That would retain backwards compatibility, but bring joy back into the life of pydocstyle/pre-commit users.

[bszonye]

Has this behavior changed again? I'm trying to reproduce a configuration problem, and I am finding that pydocstyle honors the match option in pyproject.toml even when I pass a filename on the command line.

With this config:

[tool.pydocstyle]
match = '.*\.py'

pydocstyle does check my tests:

$ pydocstyle tests/test_pmf.py
tests/test_pmf.py:16 in public class `TestPMFInit`:
        D101: Missing docstring in public class

but with the default config, pydocstyle 6.2.3 is ignoring unmatched files even when I list them explicitly on the command line:

$ pydocstyle tests/test_pmf.py
# OK

[bszonye]

After a bit of digging, I believe that this issue was resolved by #550 (released in version 6.2.0).

If I revert to pydocstyle 6.1.1, I see inconsistent behavior depending on whether the command-line argument is a filename or a path:

$ pydocstyle ./test_pmf.py
./test_pmf.py:16 in public class `TestPMFInit`:
        D101: Missing docstring in public class
$ pydocstyle test_pmf.py
# OK

After #550, those two cases behave the same:

$ pydocstyle ./test_pmf.py
# OK
$ pydocstyle test_pmf.py
# OK

Therefore, I think this issue can be closed. This also means that the reverted change in #610 wasn't actually necessary (which is good, because of the performance concerns mentioned by @danmoz).