hhatto / autopep8

A tool that automatically formats Python code to conform to the PEP 8 style guide.
https://pypi.org/project/autopep8/
MIT License
4.58k stars 291 forks source link

Search upwards in directory structure for config file (or possibly that the .flake8 exclude is not found?) #492

Open thernstig opened 5 years ago

thernstig commented 5 years ago

The documentation says:

By default, if $HOME/.config/pycodestyle (~.pycodestyle in Windows environment) exists, it will be used as global configuration file. Alternatively, you can specify the global configuration file with the --global-config option.

Also, if setup.cfg, tox.ini, .pep8 and .flake8 files exist in the directory where the target file exists, it will be used as the configuration file.

pep8, pycodestyle, and flake8 can be used as a section.

Other linters (or formatters) look upwards in the directory hiearachy for a config file in case one does not exist in the current directory. This is how it works for flake8, ESLint, TSLint and many other major linters. I expected autopep8 to work the same which made me open a (faulty) issue https://github.com/microsoft/vscode-python/issues/6505 on the VS Code Python extension, as I thought this was a problem with the extension when it in fact seems to be a missing feature of autopep8.

It would be great if autopep8 could look upwards (maybe 10 levels) in the directory structure for a config file and select the closest one.

EDIT I have not read all the code, but it seems https://github.com/hhatto/autopep8/blob/master/autopep8.py#L3933 does search upwards. Could it be that the exclude pattern for pycodestyle and flake8 is the one that differ and that the ignore pattern of flake8 is not honored (which it should be)? Here is an example:

[flake8]
exclude =
  project/some/path/here/file.py,

More info in the thread below.

stinos commented 3 years ago

This has been fixed by now, no? Just not updated in the documentation.

thernstig commented 3 years ago

@stinos In which PR/commit?

stinos commented 3 years ago

@thernstig not sure, but see https://github.com/hhatto/autopep8/blob/master/autopep8.py#L3933 where it walks upwards in the hierarchy of the file to format. So if you format /path/to/a.py it will look in /path/to for a .pep8 for example, then in /path, etc, until found. I realize now that is perhaps not exactly what you are asking for?

thernstig commented 3 years ago

I checked git blame on that line, and it was 7 years ago it was written. Either that is something else, or the implementation does not work.

stinos commented 3 years ago

I stepped through the code with a debugger and it does seem to do what it should, at least for a couple of files I tried. However it also depends on hoe autopep8 is used. I ran it directlry from the commandline and that's ok, but I came here originally thinking this did not work, but that was because I was testing it via the AutoPEP8 Sublime Text plugin: it invokes autopep8 in a different way leading it to not find config files in e.g. the files' directory itself.

thernstig commented 3 years ago

@stinos are you saying this is a problem in how e.g. VS Code invokes autopep8? Maybe you can read https://github.com/microsoft/vscode-python/issues/6505 and see the context for where this issue was created.

edit: This is how VS Codes invokes autopep8 as an example:

~/code/project/.venv/bin/python ~/.vscode-server/extensions/ms-python.python-2021.2.582707922/pythonFiles/pyvsc-run-isolated.py autopep8 --diff ~/code/project/some/path/here/file.py cwd: ~/code/project

stinos commented 3 years ago

are you saying this is a problem in how e.g. VS Code invokes autopep8

Not sure, but it's not unlikely. I just tested it here and works ok for me. Also no difference between running autpep8 via commandline, VSCode or SublimeText (after patching AutoPEP8). They all find the .pep8 I want. I'm on Windows, not sure if that should matter.

VSCode settings (none, because I want to specify everythin in .pep8 files):

"python.formatting.provider": "autopep8",
"python.formatting.autopep8Args": [],

I have a .pep8 file in c:\Users\user\Projects:

[pycodestyle]
ignore = E301,E302,E309
max-line-length = 120
aggressive = 1

Output of running autopep8 via Ctrl-Shift-R in VSCode:

C:\tools\Miniconda3\python.exe c:\Users\user\.vscode\extensions\ms-python.python-2021.2.582707922\pythonFiles\pyvsc-run-isolated.py autopep8 --diff c:\Users\user\Projects\subA\subB\somefile.py
cwd: c:\Users\user\Projects\subA

(cwd just depends on which folder is opened) And that finds and uses c:\Users\user\Projects.pep8.

Since this cannot be reproduced easily, it would be good if you provide extra information here: where is your .pep8 file? And/or you have a global config overriding things? Is there no formatting at all or what is the exact problem you have? (note VSCode's python output doesn't always produce errors in case formatting fails, e.g. I was testing this and incorrectly used "python.formatting.autopep8Args": ["-v -v"] and VSCode just doesn't format nor complains)

thernstig commented 3 years ago

Just tested again. It does not work. The initial description has to be read in context of the VS Code issue written first.

~/code/project/ is the root of the project.

I have a ~/code/project/.flake8 file with this (but also tested with a setup.cfg):

[flake8]
exclude =
  project/some/path/here/file.py,

It gives this output for the Python extension in VS Code (just so we can see how it executes autopep8):

~/code/project/.venv/bin/python ~/.vscode-server/extensions/ms-python.python-2021.2.582707922/pythonFiles/pyvsc-run-isolated.py autopep8 --diff ~/code/project/some/path/here/file.py
cwd: ~/code/project

.vscode/settings.json (the workspace settings):

  "[python]": {
    "editor.defaultFormatter": "ms-python.python"
  },
  "python.analysis.typeCheckingMode": "basic",
  "python.analysis.completeFunctionParens": true,
  "python.autoComplete.addBrackets": true,
  "python.languageServer": "Pylance",
  "python.linting.pylintEnabled": false,
  "python.linting.flake8Enabled": true,
  "python.pythonPath": "${workspaceFolder}/.venv",
  "python.sortImports.path": "${workspaceFolder}/.venv/bin/isort",
  "python.testing.pytestArgs": ["some/path"],
  "python.testing.pytestEnabled": true,

The file is not ignored. It is formatted by VS Code when I run the command Format Document. This autopep8 does not respect .flake8 config in the root of the project, as it says it should.

stinos commented 3 years ago

exclude = project/some/path/here/file.py

Now we're getting somewhere. I can reproduce this (irregardless of how autopep8 is used), but what happens (for me) is that the .flake8 is in fact found and respected, but your exclude pattern is wrong. Can you try another setting, or else use a proper glob pattern like `*project/some/path/here/file.py?

thernstig commented 3 years ago

What do you mean "proper globpattern"? The ignore pattern is perfectly valid in terms of using flake8. See https://flake8.pycqa.org/en/latest/user/configuration.html

thernstig commented 3 years ago

In addition, I can reproduce this without VS Code via CLI:

exclude in .flake8 is honored

autopep8 project/some/path/here/file.py

exclude in .flake8 is not honored

cd project/some/path/here/
autopep8 file.py
stinos commented 3 years ago

The ignore pattern is perfectly valid in terms of using flake8

Sure, but this is autopep8 and apparently it requires a full match. So I suggest you close this issue and open a new one describing the actual problem (the handling of the exclude argument). Searching upwards for config files isn't the problem here.

thernstig commented 3 years ago

@stinos did you miss the documentation that says this?

Also, if setup.cfg, tox.ini, .pep8 and .flake8 files exist in the directory where the target file exists, it will be used as the configuration file.

How else should one configure this? If we use flake8 we cannot mix exclude using various ignore patterns. It is what is is for flake8.

stinos commented 3 years ago

did you miss the documentation that says this?

No, I know all of that.

My point is: this issue claims auatopep8 does not scan upwards for configfiles. But that is not true, it does.

The problem you are having is that it treats the exclude parameter in a different way than what you expect, or what flake8 does, etc. As such it looks like you thought autopep8 didn't find your config file, while in fact it does, it just doesn't handle this particular parameter from it. As such this issue title is wrong. Hence my suggestion to create an issue about what the actual problem is.

thernstig commented 3 years ago

I can agree to that! I opposed that you mentioned I should change my ignore pattern. It might find the .flake8 as you say then, but does not honor e.g. exclude.

Is there more it does not honor? Tried to add this:

ignore =
  E303
autopep8 --diff project/some/path/here/file.py

cd project/some/path/here/
autopep8 --diff file.py

Is this also a problem with it just not honoring ignore or a problem with it not honoring .flake8 when I use the --diff flag? When run through flake8 it is honored though, as I assume flake8 won't even sent it to autopep8 to format.

stinos commented 3 years ago

you mentioned I should change my ignore pattern.

Yeah, but just to test whether that works :)

Is this also a problem with it just not honoring ignore or a problem with it not honoring .flake8 when I use the --diff flag?

No idea really. Works ok for me. Perhaps run with -v to get more output.

E.g I have /path/to/foo.py with too much blank lines in it, and /path/.flake8 which is empty, then

>> autopep8 -v --diff /path/to/foo.py
...
1 issue(s) to fix {'E303': {8}}
...

Then I change /path/.flake8` to

[flake8]
ignore =
 E303

and that finds the file and applies the options:

>> autopep8 -v --diff /path/to/foo.py
enable config: section=flake8, key=ignore, value=
E303
...
0 issue(s) to fix {}
...
thernstig commented 3 years ago

Well it did not work for me, but not going derail this further, but I will update the main post about just "ignore" for now so that gets focused on.

thernstig commented 3 years ago

@hhatto Do you have anything to add to the new findings (read thread and newly updated main post).