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

E203: False positive "whitespace before ':' " on list slice. #373

Open
aleksey-kutepov opened this issue Jan 21, 2015 · 24 comments · May be fixed by #1047
Open

E203: False positive "whitespace before ':' " on list slice. #373

aleksey-kutepov opened this issue Jan 21, 2015 · 24 comments · May be fixed by #1047

Comments

@aleksey-kutepov
Copy link

I've encountered the problem in the following code:

a = [1, 2, 3, 4, 5]
b = a[1+1 : 2+2]  # E203
c = a[1 + 1 : 2 + 2]  # E203
d = a[1+1:2+2]

However, PEP8 chapter https://www.python.org/dev/peps/pep-0008/#whitespace-in-expressions-and-statements handles this as good style:

However, in a slice the colon acts like a binary operator, and should have equal amounts on either side (treating it as the operator with the lowest priority). In an extended slice, both colons must have the same amount of spacing applied. Exception: when a slice parameter is omitted, the space is omitted.

Yes:

ham[1:9], ham[1:9:3], ham[:9:3], ham[1::3], ham[1:9:]
ham[lower:upper], ham[lower:upper:], ham[lower::step]
ham[lower+offset : upper+offset]
ham[: upper_fn(x) : step_fn(x)], ham[:: step_fn(x)]
ham[lower + offset : upper + offset]
No:

ham[lower + offset:upper + offset]
ham[1: 9], ham[1 :9], ham[1:9 :3]
ham[lower : : upper]
ham[ : upper]

@catgeek86
Copy link

I took a look at the code and it looks like the function extraneous_whitespace
on line 271 in pep8.py is the function that needs to be updated. It is
returning "E03 whitespace before :" regardless of whether it is a legitimate.

This is confirmed in the doc string for the function, showing that E203 it covers these cases:
E203: if x == 4: print x, y; x, y = y , x
E203: if x == 4: print x, y ; x, y = y, x
E203: if x == 4 : print x, y; x, y = y, x

Looking at the testsuite E20.py confirmed that it is not accounting for legitimate uses of this case.

@catgeek86
Copy link

This needs to be cleaned up A LOT but here is a prototype of a potential fix(very incomplete but it is a workable idea)
it recognizes ham[lower + offset : upper + offset] as correct and [1 : 2] as incorrect.

Please keep in mind this was a quick very incomplete prototype.
If no one else has a better idea, then I would like to cleanup/finish this idea.

  1. a regex to recognize the colon used as an operator (this needs to be cleaned up and needs operators other than '+' added)
    COLON_AS_OPERATOR = re.compile(r'\S+\s_+\s_\S+\s:\s\S+\s_+\s_\S+')

  2. within the extraneous_whitespace function, it calls a new function to check if the colon is being used as an operator
    if char == ':':
    colon_is_operator = is_colon_operator(logical_line)

  3. The new function is_colon_operator checks to see if it matches the regex as defined
    in COLON_AS_OPERATOR
    def is_colon_operator(logical_line):
    found = 0
    for match in COLON_AS_OPERATOR.finditer(logical_line):
    found = 1
    return found

  4. if the colon is being used as an operator, it does not return an error. Otherwise (as in [1 : 2])
    it returns the 203 error
    if colon_is_operator == False:
    yield found, "%s whitespace before '%s'" % (code, char)

@vodik
Copy link
Contributor

vodik commented Jun 3, 2016

So I've been hacking on this for some time, and I don't know how to approach this without tokenizing. The wall I'm hitting is that the rules are context sensitive.

I've managed to hack the linter enough that it handles expressions inside [] specially. However that breaks when you have lists of dictionaries, where the rules "revert" back inside {}.

I have some code that somewhat works, but I'd be interested in some more guidance on how to tackle this.

@jayvdb
Copy link
Member

jayvdb commented Jun 3, 2016

Hi @vodik, you can add a parameter tokens, and if you need state (im not sure it is needed) see use of checker_state in module_imports_on_top_of_file.

@RonnyPfannschmidt
Copy link

this is now triggered by black doing the "right" thing - psf/black#279

@sigmavirus24
Copy link
Member

@RonnyPfannschmidt I'd be happy to merge a PR that fixes this case

@RonnyPfannschmidt
Copy link

@sigmavirus24 i#ll takea short look if i can figure a quickfix

@RonnyPfannschmidt
Copy link

no quickfix for this one :(

@webknjaz
Copy link

Facing the same issue:

-        attr_name = adapter[last_dot + 1:]
+        attr_name = adapter[last_dot + 1 :]

@tommilligan-plutoflume
Copy link

As a temporary workaround, black will format the following without spacing (passing pycodestyle check):

i = last_dot + 1
attr_name = adapter[i:]

@RonnyPfannschmidt
Copy link

i simply disabled the check, as black is making those bits correct in all cases

mosbasik added a commit to mosbasik/dotfiles that referenced this issue Oct 23, 2018
peterjc added a commit to peterjc/thapbi-pict that referenced this issue Jan 9, 2019
Using black's default line length of 88.

Black follows the PEP8 standard more closely than
pycodestyle does (as used in the flake8 checks)
so must disable E203 until this is fixed:
PyCQA/pycodestyle#373
peterjc added a commit to peterjc/thapbi-pict that referenced this issue Jan 10, 2019
Using black's default line length of 88.

Black follows the PEP8 standard more closely than
pycodestyle does (as used in the flake8 checks)
so must disable E203 until this is fixed:
PyCQA/pycodestyle#373
jonathanhaigh-arm added a commit to PelionIoT/mbl-tools that referenced this issue Jan 29, 2019
* The default behaviour of pydocstyle is to ignore "test_" files when
  given a directory to check. Follow its example and ignore "test_"
  files in the invocation we use too.

* Ignore pycodestyle "E203" warnings - sometimes they are spurious
  (PyCQA/pycodestyle#373).
jonathanhaigh-arm added a commit to PelionIoT/mbl-tools that referenced this issue Jan 29, 2019
* The default behaviour of pydocstyle is to ignore "test_" files when
  given a directory to check. Follow its example and ignore "test_"
  files in the invocation we use too.

* Ignore pycodestyle "E203" warnings - sometimes they are spurious
  (PyCQA/pycodestyle#373).
kimt33 added a commit to theochem/fanpy that referenced this issue Apr 12, 2019
What:
1. Add exception for E203 whitespace before ':'
2. Ignore use of bad variable name "l"
3. Remove unused import

Why:
1. There seems to be a discrepancy between the use of ':' within a slice:
PyCQA/pycodestyle#373. Since black can handle the
whitespace regarding ':' for both in and out of slices, we ignore this error and
trust black to format it.
2. Because l is comonly used in equations for density matrices
@cclauss
Copy link

cclauss commented May 6, 2019

Is there a way to align the projects pycodestyle and black so that users no longer need to manually configure them to work on a common repo? This alignment is now more important given that black has been moved into an official python repo: https://github.com/python/black

@peterjc
Copy link
Contributor

peterjc commented May 6, 2019

@cclauss did you mean aside from fixing this issue (#373 in pycodestyle)?

Slightly tangental, but I wrote flake8-black to let me run the black style checks as well as the pycodestyle checks from within flake8, see:

https://github.com/peterjc/flake8-black

I had to document ignoring E203 as a workaround.

@asottile
Copy link
Member

asottile commented May 6, 2019

Is there a way to align the projects pycodestyle and black so that users no longer need to manually configure them to work on a common repo? This alignment is now more important given that black has been moved into an official python repo: https://github.com/python/black

fwiw the move into the python project is because it is a PSF project now

From this:

Q: Does it mean it's official now?
A: No, just like mypy isn't.

s-weigand added a commit to s-weigand/python-holidays that referenced this issue Oct 17, 2020
E203 needs to be ignore due to a false positive alert for slicing. See. PyCQA/pycodestyle#373 (comment)
s-weigand added a commit to s-weigand/python-holidays that referenced this issue Oct 17, 2020
E203 needs to be ignore due to a false positive alert for slicing. See. PyCQA/pycodestyle#373 (comment)
s-weigand added a commit to s-weigand/python-holidays that referenced this issue Oct 17, 2020
E203 needs to be ignore due to a false positive alert for slicing. See. PyCQA/pycodestyle#373 (comment)
s-weigand added a commit to s-weigand/python-holidays that referenced this issue Dec 4, 2020
E203 needs to be ignore due to a false positive alert for slicing. See. PyCQA/pycodestyle#373 (comment)
s-weigand added a commit to s-weigand/python-holidays that referenced this issue Dec 4, 2020
E203 needs to be ignore due to a false positive alert for slicing. See. PyCQA/pycodestyle#373 (comment)
s-weigand added a commit to s-weigand/python-holidays that referenced this issue Dec 7, 2020
E203 needs to be ignore due to a false positive alert for slicing. See. PyCQA/pycodestyle#373 (comment)
s-weigand added a commit to s-weigand/python-holidays that referenced this issue Dec 7, 2020
E203 needs to be ignore due to a false positive alert for slicing. See. PyCQA/pycodestyle#373 (comment)
@FabianNiehaus
Copy link

I still have this issue with flake8 v.3.8.3.

When definining a string slice like this:

my_string[my_string.rindex("/") + 1 : my_string.index(":")]

flake8 will raise violation E203.

However, this should not apply to slices, as mentioned in https://www.python.org/dev/peps/pep-0008/#pet-peeves.

Manually removing the spaces does not work in our case, as code formatting is done by black, which will automatically add whitepspaces around : when used in a slice.

dr-prodigy added a commit to vacanza/holidays that referenced this issue Feb 24, 2021
…443)

* added coverage.xml to .gitignore

* Basic rewrite of the travis tests, with github actions

* removed now obsolete travis config

* Added bump2version and its config

* moved flake8 config to setup.cfg

* Formatted requirements with comments

* Created test file for each country and moved corresponding tests there

* Added pre-commit and a basic config containing black+its config

* Added flake8 to pre-commit config and set flake8 to ignore E203

E203 needs to be ignore due to a false positive alert for slicing. See. PyCQA/pycodestyle#373 (comment)

* Replaced flake8 tests with pre-commit ones, since flake8 is included

* Updated workflow to use tests folder, upload coverage to coveralls.io 

and some minor fixes

* Added pre-commit hook that adds encoding shebang to all files

py27 compat

* Added coverage config

* Added tox+pytest config and added .tox to .gitignore

* Added rst checking tools  to pre-commit and fixed rst issues

* Added workflow dispatch as an option to run the workflow

* Removed py35 as was done in https://github.com/dr-prodigy/python-holidays/pull/402

* Added dependabot config to receive automatics update PR's 

for python runtime dependencies and used github-actions

* Fixed pytest config

quoting the addopts made them not being recognized properly

* Replaced travis with github actions badge

* Changed contributing guide to reflect changed tooling

* Formatted setup.py again with black

* Auto-format, Israel to_jd_gregorianyear fix

* requirements_dev.txt review, Israel fixes, CHANGES+README.rst reviews

* precommit tasks run

* removed python 2.7 checks from github CI/CD scripts

* flake8 config fixes

* travis build toml dependency fix

* removed duplicated flake8 tests from coverage config

* old tests.py using new test classes

* tests tree reviewed

* tests cleanup (warn: coverage report -m needs fixing)

* Fixed usage of coveralls with gh-action after 3.0.0 release

* more recent tests re-applied - #404

* copyright 2021

* test tree refactoring, pytest running thru tests.py

* Flake8 test removed, pyproject.toml cleanup

* Added .gitaddtributes file to ensure consistent '\n' line ending

for future commits

* Added pre-commit hook to enforce '\n' lineending and applied it on all

files

* Copied files from 'holiday' and 'tests' over and ran 'pre-commit run -a'

This should catch all changes on beta which were missing.
I left changes which were added in the cleanup of 'unstable' e.g. 'holidays.utils.is_leap_year'

'test/countries/test_saudiarabia.py' was renamed to 'test/countries/test_saudi_arabia.py' to be consistent with 'holidays/countries/saudi_arabia.py'

* is_leap_year removal

Co-authored-by: s-weigand <[email protected]>
dr-prodigy added a commit to vacanza/holidays that referenced this issue Feb 24, 2021
(massive thx+kudos to s-weigand)

* added coverage.xml to .gitignore

* Basic rewrite of the travis tests, with github actions

* removed now obsolete travis config

* Added bump2version and its config

* moved flake8 config to setup.cfg

* Formatted requirements with comments

* Created test file for each country and moved corresponding tests there

* Added pre-commit and a basic config containing black+its config

* Added flake8 to pre-commit config and set flake8 to ignore E203

E203 needs to be ignore due to a false positive alert for slicing. See. PyCQA/pycodestyle#373 (comment)

* Replaced flake8 tests with pre-commit ones, since flake8 is included

* Updated workflow to use tests folder, upload coverage to coveralls.io

and some minor fixes

* Added pre-commit hook that adds encoding shebang to all files

py27 compat

* Added coverage config

* Added tox+pytest config and added .tox to .gitignore

* Added rst checking tools  to pre-commit and fixed rst issues

* Added workflow dispatch as an option to run the workflow

* Removed py35 as was done in https://github.com/dr-prodigy/python-holidays/pull/402

* Added dependabot config to receive automatics update PR's

for python runtime dependencies and used github-actions

* Fixed pytest config

quoting the addopts made them not being recognized properly

* Replaced travis with github actions badge

* Changed contributing guide to reflect changed tooling

* Formatted setup.py again with black

* Auto-format, Israel to_jd_gregorianyear fix

* requirements_dev.txt review, Israel fixes, CHANGES+README.rst reviews

* precommit tasks run

* removed python 2.7 checks from github CI/CD scripts

* flake8 config fixes

* travis build toml dependency fix

* removed duplicated flake8 tests from coverage config

* old tests.py using new test classes

* tests tree reviewed

* tests cleanup (warn: coverage report -m needs fixing)

* Fixed usage of coveralls with gh-action after 3.0.0 release

* more recent tests re-applied - #404

* copyright 2021

* test tree refactoring, pytest running thru tests.py

* Flake8 test removed, pyproject.toml cleanup

* Added .gitaddtributes file to ensure consistent '\n' line ending

for future commits

* Added pre-commit hook to enforce '\n' lineending and applied it on all

files

* Copied files from 'holiday' and 'tests' over and ran 'pre-commit run -a'

This should catch all changes on beta which were missing.
I left changes which were added in the cleanup of 'unstable' e.g. 'holidays.utils.is_leap_year'

'test/countries/test_saudiarabia.py' was renamed to 'test/countries/test_saudi_arabia.py' to be consistent with 'holidays/countries/saudi_arabia.py'

* is_leap_year removal

Co-authored-by: s-weigand <[email protected]>
boulch added a commit to IMIO/imio.directory.core that referenced this issue May 20, 2021
@jeffteixeira
Copy link

Any news?

@asottile
Copy link
Member

@jeffteixeira do you see a comment or resolution here? bumping the thread does not help

@PyCQA PyCQA locked as spam and limited conversation to collaborators Dec 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.