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

Update pyflakes to version 2.2.0 #884

Closed
Numerlor opened this issue Apr 17, 2020 · 10 comments · Fixed by #947
Closed

Update pyflakes to version 2.2.0 #884

Numerlor opened this issue Apr 17, 2020 · 10 comments · Fixed by #947
Labels
a: dependencies Related to package dependencies and management p: 2 - normal Normal Priority

Comments

@Numerlor
Copy link
Contributor

The new version introduces support for assignment expressions (changelog).
Currently when lint is ran with the expression in the code, pyflakes will crash flake8.
Updating the version fixes this but a noqa ignore is still necessary because pycodestyle did not yet receive an update and does not recognize the new syntax with a colon.

The update will introduce these lint errors: https://paste.fuelrats.com/rotukasohe.txt
where F999s are f-strings with no expressions inside of them and the F821s referring to this method definition's literals in the calls inside the typehints

async def infraction_edit(
self,
ctx: Context,
infraction_id: t.Union[int, allowed_strings("l", "last", "recent")],
duration: t.Union[Expiry, allowed_strings("p", "permanent"), None],
*,
reason: str = None
) -> None:

@kwzrd kwzrd added a: dependencies Related to package dependencies and management p: 2 - normal Normal Priority s: planning Discussing details labels Apr 17, 2020
@kwzrd
Copy link
Contributor

kwzrd commented Apr 17, 2020

How do we feel about F999? I recall a discussion in #dev-contrib regarding multi-line strings such as:

string = (
    f"First line with some {param} "
    f"second line without params"
)

If I remember correctly, the majority of our core devs are in favour of keeping the second f as it aligns the literals.

EDIT: I also believe some of us feel strongly against using the walrus what-so-ever.

@Numerlor
Copy link
Contributor Author

Looks like the joined f-strings are taken care of, taking your snippet we only get an error about param

D:\pycon>pyflakes splitfstrings.py
splitfstrings.py:2:29 undefined name 'param'

I don't know about the walrus but it can be nice to use in some simple cases without sacrificing any readability.
The pyflakes update still brings useful things so if the walrus is decided against; a notice could be posted or the flake8-walrus package can be used which forbids them.

@kwzrd
Copy link
Contributor

kwzrd commented Apr 17, 2020

Looks like the joined f-strings are taken care of ...

That's good to hear, thanks for the clarification.

For the record, I'm not opposed to this. I'm just hoping to get a discussion started to see whether we're all collectively on board.

I like the walrus myself, and I recently bumped pyflakes to 2.2.0 in one of my projects. It is, however, disappointing that even then, the lines still need to be noqa to avoid the problems you mentioned.

@kwzrd
Copy link
Contributor

kwzrd commented Apr 18, 2020

Sorry, I did not properly read yesterday and didn't realize pyflakes has had a release. I thought we'd still have to grab the specific commit the adds assignment expression support. Let's do this then, I don't see a reason to wait, and apologies again.

@kwzrd kwzrd removed the s: planning Discussing details label Apr 18, 2020
@kwzrd
Copy link
Contributor

kwzrd commented Apr 18, 2020

Good news is that pycodestyle already supports walrus on master and is only waiting on a proper release: PyCQA/pycodestyle#911.

@Numerlor
Copy link
Contributor Author

Ah, looks like flake8 is incompatible with the newest pyflakes and the above missing placeholder error codes are wrong because of that, the only thing incompatible changes are the error codes which were fixed in this merge commit thatfixes it by simply updating a dict PyCQA/flake8@6efb15c

Do we care about the error codes enough to delay this until a release? Couldn't find any hints on when that may be.

@kwzrd
Copy link
Contributor

kwzrd commented Apr 20, 2020

Hmm, I'd probably lean towards just waiting on a proper flake8 release then.

@sco1
Copy link
Contributor

sco1 commented Apr 24, 2020

Let's just wait for the flake8 3.8.0 release. The alpha is out and it doesn't make sense to add a bunch of manual dependency pins just to chase the walrus.

@sco1
Copy link
Contributor

sco1 commented May 11, 2020

flake8 3.8 is now released: https://pypi.org/project/flake8/3.8.0/

@lemonsaurus
Copy link
Member

flake8 version will be bumped to 3.8 in #947

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a: dependencies Related to package dependencies and management p: 2 - normal Normal Priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants