Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

autopep8 #551

Open
jayvdb opened this issue Apr 10, 2018 · 10 comments
Open

autopep8 #551

jayvdb opened this issue Apr 10, 2018 · 10 comments
Assignees

Comments

@jayvdb
Copy link
Member

jayvdb commented Apr 10, 2018

Lots of opportunity to add fixes.

@jayvdb
Copy link
Member Author

jayvdb commented May 5, 2018

@AbdealiJK , hhatto/autopep8#227 gives me an idea that might be a good basis for a GSoC size project.

We can already tell autopep8 which problems to fix using a combination of --select and --line-range.

Therefore, we could let pycodestyle do the error detection, with the usual 'coala ignore' processing and the PycodestyleBear settings, and then pass only the resulting filtered errors to autopep8 to generate fixes only for.

The fixes could then be applied by the bear and pycodestyle re-run on the file to verify that the fixes satisfied the pycodestyle rule, and bad fixes rejected.

This might be done by chaining PycodestyleBear and PEP8Bear together, would involve serious enhancements to coala core, or PycodestyleBear could be enhanced to also use autopep8 to get fixes. Even if the former approach is designed well, it still may not be a very re-usable as it would depend on two bears having the same codes for a problem. That does occur in other languages, but probably not very frequently. (I know the markdown style tools have a set of similar codes used across different programs written in Python and Node). Invoking autopep8 in PycodestyleBear would still likely 'require' some improvements to coala core, to create fixes for error reports, but they would be less complicated.

Then the autopep8 part of the project would be to add the ability to specify column numbers with --range in addition to line numbers.

@areebbeigh
Copy link
Member

areebbeigh commented Jan 31, 2019

Adding a few things:

We can safely assume that autopep8 won't expand to Pycodestyle-like error reporting capabilities anytime soon.

There already was an attempt to use Pycodestyle and autopep8 together but this approach tries to run Pycodestyle alongside autopep8 and assumes that errors detected by Pycodestyle will be in the same order as autopep8 and equal to the set of errors detected by the latter.

Pycodestyle (the library) is solely focused on detecting PEP8 violations and not fixing them. We might want to leave that behaviour as is in its bear. Instead, creating a new bear might be a better way to implement this.

@areebbeigh
Copy link
Member

Then the autopep8 part of the project would be to add the ability to specify column numbers with --range in addition to line numbers.

This shouldn't be required with the following workflow:

  • Run pycodestyle on the code
  • Run autopep8 with --select <error detected by pycodestyle> on the line with the issue

I don't see why implementing --col-range should be necessary.

@bkhanale
Copy link
Member

bkhanale commented Mar 3, 2019

I don't see why implementing --col-range should be necessary.

This is because one single line could've multiple same types of errors. Through --column-range we could select the ones for which we need a fix.

Example: If the file contains:

print( sys.path, ( 2))

Then the error would be:

test.py:1:7: E201 whitespace after '('
print( sys.path, ( 2))
      ^
test.py:1:19: E201 whitespace after '('
print( sys.path, ( 2))

By running autopep8 with --select=E201 it would fix both of the errors. But by specifying column-range we could select one of them, giving even more control.

@areebbeigh
Copy link
Member

areebbeigh commented Mar 3, 2019

By running autopep8 with --select=E201 it would fix both of the errors. But by specifying column-range we could select one of them, giving even more control.

Yes, but PycodestyleBear and PEP8Bear generate only one result for a set of line-error pairs:

bad.py
[   5] print(·'test'·)·
**** PycodestyleBear (E201) [Section: cli | Severity: NORMAL] ****
!    ! E201 whitespace after '('
[    ] *0. Do (N)othing
[    ]  1. (O)pen file
[    ]  2. Add (I)gnore comment
[    ] Enter number (Ctrl-D to exit): 
!    ! The code does not comply to PEP8.
[----] /home/areebbeigh/Documents/sandbox/Python/linters/bad.py
[++++] /home/areebbeigh/Documents/sandbox/Python/linters/bad.py
[   5] print( 'test' ) 
[   5] print('test')
[    ] *0. Do (N)othing
[    ]  1. (O)pen file
[    ]  2. (A)pply patch
[    ]  3. Add (I)gnore comment
[    ]  4. Show Applied (P)atches
[    ]  5. (G)enerate patches
[    ] Enter number (Ctrl-D to exit): 

So we wouldn't really be using column-range anywhere in the bears

@bkhanale
Copy link
Member

bkhanale commented Mar 3, 2019

The improvement would be for the autopep8 rather than for the bears. An attempt was already made by a GCI student hhatto/autopep8#452.

@areebbeigh
Copy link
Member

Hm yeah, looks like column range is indeed needed here. It's only the PEP8Bear that patches multiple problems in a line at once, PycodestyleBear reports them one by one.

Related: coala/coala-bears#2882

@xurror
Copy link

xurror commented Mar 22, 2019

Please what's the status for this project

@xurror
Copy link

xurror commented Mar 22, 2019

@jayvdb will you mentor this project this year?

I would like to work on this

@bkhanale
Copy link
Member

This issue is reserved under the GSoC project Improve Generic Bear Quality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants