Change Python project root dir detection for flake8 configuration #2502

ericvw · 2019-05-14T02:54:23Z

The official configuration files for flake8 are .flake8, tox.ini,
and setup.cfg.

After investigation, it is safe to remove flake8.cfg as it appears to
only exist as a typo in other tooling documentation (e.g.,
python-language-server).

Even though no linters automatically read .flake8rc, it is kept in
case projects may be using it for detecting the projects root directory.

http://flake8.pycqa.org/en/latest/user/configuration.html#configuration-locations

w0rp

Add a test for this.

w0rp · 2019-05-14T08:28:45Z

autoload/ale/python.vim

@@ -25,8 +25,7 @@ function! ale#python#FindProjectRootIni(buffer) abort
        \|| filereadable(l:path . '/tox.ini')
        \|| filereadable(l:path . '/mypy.ini')
        \|| filereadable(l:path . '/pycodestyle.cfg')
-        \|| filereadable(l:path . '/flake8.cfg')


python-language-server discovers paths by looking for flake8.cfg, so I'd keep this. It's mentioned here: https://pypi.org/project/python-language-server/

It appears to be a typo in the README of python-language-server.

https://github.com/palantir/python-language-server/blob/96e08d85635382d17024c352306c4759f124195d/pyls/config/flake8_conf.py#L10

The commit which introduced the original introduced the change for reading flake8 but documented it incorrectly (palantir/python-language-server@3c1f183).

The python-language-server has limited contributions to the project to those who have contributed in the past — I am unable to open an issue nor a pull request to address this discrepancy 😞 .

w0rp · 2019-05-14T08:30:37Z

autoload/ale/python.vim

@@ -25,8 +25,7 @@ function! ale#python#FindProjectRootIni(buffer) abort
        \|| filereadable(l:path . '/tox.ini')
        \|| filereadable(l:path . '/mypy.ini')
        \|| filereadable(l:path . '/pycodestyle.cfg')
-        \|| filereadable(l:path . '/flake8.cfg')
-        \|| filereadable(l:path . '/.flake8rc')


A number of projects and linter plugins search for .flake8rc, so I'd keep this too.

I searched the history of the flake8 project and it has never supported .flake8rc, from what I can tell.

I conducted a hasty Google search: ".flake8rc" and identified the following (potential) projects that may be using this are:

https://github.com/AtomLinter/linter-flake8

https://github.com/flycheck/flycheck

https://github.com/w0rp/ale

Ale is using this in the documentation and within this function.

For linter-flake8, it looks like it is another documentation bug (https://github.com/AtomLinter/linter-flake8/blob/875c273d3affe253478b804a51441688a0b2d03e/package.json#L27), from search https://github.com/AtomLinter/linter-flake8/search?q=flake8rc&unscoped_q=flake8rc.

For flycheck, it appears they have a flycheck-flake8rc-specific configuration file documented in (https://github.com/flycheck/flycheck/blob/47174a12fd84a685f36019632838c73b4813b66d/doc/languages.rst).

w0rp · 2019-05-14T08:32:03Z

autoload/ale/python.vim

@@ -25,8 +25,7 @@ function! ale#python#FindProjectRootIni(buffer) abort
        \|| filereadable(l:path . '/tox.ini')
        \|| filereadable(l:path . '/mypy.ini')
        \|| filereadable(l:path . '/pycodestyle.cfg')
-        \|| filereadable(l:path . '/flake8.cfg')
-        \|| filereadable(l:path . '/.flake8rc')
+        \|| filereadable(l:path . '/.flake8')


You can move this further up the list, as this is the officially supported filename.

ericvw

Add a test for this.

Absolutely.

I noticed that the tests for ale#python#FindProjectRoot are in https://github.com/w0rp/ale/blob/master/test/command_callback/test_flake8_command_callback.vader. The path of least resistence would be to add a test there, which I'm planning on doing.

There is an opportunity to factor these out into a separate file, which occurs for some other languagse. If you don't mind, I'm happy to do this refactoring as a separate PR.

I'm still getting familiar with Vimscript, so it may take me a bit to get this right 😄.

@w0rp, let me know how you would like to proceed with the other configuration files that I responded to.

ericvw · 2019-05-14T13:15:46Z

autoload/ale/python.vim

@@ -25,8 +25,7 @@ function! ale#python#FindProjectRootIni(buffer) abort
        \|| filereadable(l:path . '/tox.ini')
        \|| filereadable(l:path . '/mypy.ini')
        \|| filereadable(l:path . '/pycodestyle.cfg')
-        \|| filereadable(l:path . '/flake8.cfg')


It appears to be a typo in the README of python-language-server.

https://github.com/palantir/python-language-server/blob/96e08d85635382d17024c352306c4759f124195d/pyls/config/flake8_conf.py#L10

The commit which introduced the original introduced the change for reading flake8 but documented it incorrectly (palantir/python-language-server@3c1f183).

The python-language-server has limited contributions to the project to those who have contributed in the past — I am unable to open an issue nor a pull request to address this discrepancy 😞 .

ericvw · 2019-05-14T13:41:21Z

autoload/ale/python.vim

@@ -25,8 +25,7 @@ function! ale#python#FindProjectRootIni(buffer) abort
        \|| filereadable(l:path . '/tox.ini')
        \|| filereadable(l:path . '/mypy.ini')
        \|| filereadable(l:path . '/pycodestyle.cfg')
-        \|| filereadable(l:path . '/flake8.cfg')
-        \|| filereadable(l:path . '/.flake8rc')


I searched the history of the flake8 project and it has never supported .flake8rc, from what I can tell.

I conducted a hasty Google search: ".flake8rc" and identified the following (potential) projects that may be using this are:

https://github.com/AtomLinter/linter-flake8

https://github.com/flycheck/flycheck

https://github.com/w0rp/ale

Ale is using this in the documentation and within this function.

For linter-flake8, it looks like it is another documentation bug (https://github.com/AtomLinter/linter-flake8/blob/875c273d3affe253478b804a51441688a0b2d03e/package.json#L27), from search https://github.com/AtomLinter/linter-flake8/search?q=flake8rc&unscoped_q=flake8rc.

For flycheck, it appears they have a flycheck-flake8rc-specific configuration file documented in (https://github.com/flycheck/flycheck/blob/47174a12fd84a685f36019632838c73b4813b66d/doc/languages.rst).

w0rp · 2019-05-14T14:20:31Z

Leave .flake8rc in place. flake8 has never recognised it, but there are projects out there that use it. If flake8.cfg isn't used in any project, we can remove it.

ericvw · 2019-05-14T15:08:29Z

Leave .flake8rc in place. flake8 has never recognised it, but there are projects out there that use it.

I'm curious more than anything: What other linters have you seen that use it? I haven't been able to find anything concrete that would affect the Python linters which ale calls out to.

I'll leave .flake8rc in place as requested and will update the PR once I have a test in place for detecting .flake8. Greatly appreciate the quick review feedback :).

w0rp · 2019-05-14T15:13:46Z

No linters automatically read it, but the location of the file is used to figure out where the root of a project might be.

ericvw · 2019-05-18T13:44:53Z

@w0rp, I have updated the PR with the changes requested and added a new test suite for future iterations of detecting a Python project's root directory.

Add test files and a new test suite for detecting a Python project's root directory.

The official configuration files for `flake8` are `.flake8`, `tox.ini`, and `setup.cfg`. After investigation, it is safe to remove `flake8.cfg` as it appears to only exist as a typo in other tooling documentation (e.g., `python-language-server`). Even though no linters automatically read `.flake8rc`, it is kept in case projects may be using it for detecting the projects root directory.

w0rp · 2019-05-20T23:15:17Z

Cheers! 🍻

w0rp requested changes May 14, 2019

View reviewed changes

ericvw commented May 14, 2019

View reviewed changes

ericvw changed the title ~~Fix Python flake8 configuration file detection~~ Change Python project root dir detection for flake8 configuration May 18, 2019

ericvw added 2 commits May 19, 2019 09:51

Test for Python project root directory

57b9d8b

Add test files and a new test suite for detecting a Python project's root directory.

w0rp approved these changes May 20, 2019

View reviewed changes

w0rp merged commit 89db851 into dense-analysis:master May 20, 2019

ericvw deleted the flake8-config branch May 21, 2019 01:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change Python project root dir detection for flake8 configuration #2502

Change Python project root dir detection for flake8 configuration #2502

ericvw commented May 14, 2019 •

edited

w0rp left a comment

w0rp May 14, 2019

ericvw May 14, 2019

w0rp May 14, 2019

ericvw May 14, 2019

w0rp May 14, 2019

ericvw left a comment

ericvw May 14, 2019

ericvw May 14, 2019

w0rp commented May 14, 2019

ericvw commented May 14, 2019

w0rp commented May 14, 2019

ericvw commented May 18, 2019

w0rp commented May 20, 2019

Change Python project root dir detection for flake8 configuration #2502

Change Python project root dir detection for flake8 configuration #2502

Conversation

ericvw commented May 14, 2019 • edited

w0rp left a comment

Choose a reason for hiding this comment

w0rp May 14, 2019

Choose a reason for hiding this comment

ericvw May 14, 2019

Choose a reason for hiding this comment

w0rp May 14, 2019

Choose a reason for hiding this comment

ericvw May 14, 2019

Choose a reason for hiding this comment

w0rp May 14, 2019

Choose a reason for hiding this comment

ericvw left a comment

Choose a reason for hiding this comment

ericvw May 14, 2019

Choose a reason for hiding this comment

ericvw May 14, 2019

Choose a reason for hiding this comment

w0rp commented May 14, 2019

ericvw commented May 14, 2019

w0rp commented May 14, 2019

ericvw commented May 18, 2019

w0rp commented May 20, 2019

ericvw commented May 14, 2019 •

edited