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

consider-using-f-string should not be issued with py-version < 3.6 #5019

Closed
scop opened this issue Sep 16, 2021 · 4 comments · Fixed by #5024
Closed

consider-using-f-string should not be issued with py-version < 3.6 #5019

scop opened this issue Sep 16, 2021 · 4 comments · Fixed by #5024
Assignees
Milestone

Comments

@scop
Copy link
Contributor

scop commented Sep 16, 2021

Bug description

f-strings were introduced in Python 3.6, they should not be recommended with earlier py-versions.

Configuration

No response

Command used

echo '"{}".format("foo")' | pylint --py-version=3.5 --from-stdin mymodule | grep f-string

Pylint output

mymodule:1:0: C0209: Formatting a regular string which could be a f-string (consider-using-f-string)

Expected behavior

No consider-using-f-string message when py-version is set to earlier than 3.6.

Pylint version

pylint 2.11.0
astroid 2.8.0
Python 3.9.3 (default, Apr  3 2021, 00:13:19) 
[GCC 9.3.0]

OS / Environment

No response

Additional dependencies

No response

@scop scop added Bug 🪲 Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Sep 16, 2021
scop added a commit to scop/hashpipe that referenced this issue Sep 16, 2021
pylint shouldn't be issuing this in Python < 3.6 mode.

Refs pylint-dev/pylint#5019
@Pierre-Sassoulas
Copy link
Member

Hmm, I don't think pylint is supposed to be able to analyze version < 3.6. The py-version is kinda misleading because it feels like it should work but there was clearly an assumption that python interpreter used == py-version, originally. What do you think @cdce8p ?

@Pierre-Sassoulas Pierre-Sassoulas added Discussion 🤔 and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Sep 16, 2021
@scop
Copy link
Contributor Author

scop commented Sep 16, 2021

Hm, not sure what you mean by that assumption. But FWIW the way it's put in the release notes:

Added py-version config key (if [MASTER] section). Used for version dependant checks. Will default to whatever Python version pylint is executed with.

...gives a very strong impression that it actually does work. In my opinion it definitely should :)

BTW the features entry for this option at https://docs.pylint.org/en/latest/technical_reference/features.html?highlight=py-version#general-options is quite confusing:

Min Python version to use for version dependend checks. Will default to the version used to run pylint.
Default: 3.8

It first says default is the version used to run pylint, then it says default is 3.8.
(While at it, I'd suggest rephrasing Min -> Minimum, and fixing dependend -> dependent. Would submit a PR for this but perhaps there will be bigger changes to the doc if I'm not right about what the option is for.)

@cdce8p
Copy link
Member

cdce8p commented Sep 16, 2021

Hmm, I don't think pylint is supposed to be able to analyze version < 3.6. The py-version is kinda misleading because it feels like it should work but there was clearly an assumption that python interpreter used == py-version, originally.

That was my initial though as well. However, it does make sense to extend it to other checks. If I remember correctly we discussed that shortly that users might want this setting to work for other checks as well. I haven't though about consider-using-f-string as I've disabled it for me personally, but that would indeed be a good fit. It shouldn't be too difficult either. Just the question which other checks need to support it as well. I guess new issues is one way to find out, even if unplanned.

FYI: At the moment, py-version is only used in the Typing and CodeStyle extensions.

But FWIW the way it's put in the release notes: ... ...gives a very strong impression that it actually does work. In my opinion it definitely should :)

I agree. That should have been put better into context.

It first says default is the version used to run pylint, then it says default is 3.8.

The docs are autogenerated with Python 3.8. A bit unfortunate if I'm being honest
https://github.com/PyCQA/pylint/blob/da36529a6ae1fb83417e600528692530a27aac1f/pylint/lint/pylinter.py#L475-L484

(While at it, I'd suggest rephrasing Min -> Minimum, and fixing dependend -> dependent. Would submit a PR for this but perhaps there will be bigger changes to the doc if I'm not right about what the option is for.)

Thanks for the suggestion! A PR would be welcome.

@cdce8p
Copy link
Member

cdce8p commented Sep 16, 2021

Opened #5024 that should address the main concern with the consider-using-f-string checker.

@cdce8p cdce8p self-assigned this Sep 17, 2021
@Pierre-Sassoulas Pierre-Sassoulas added this to the 2.11.2 milestone Sep 17, 2021
scop added a commit to scop/hashpipe that referenced this issue Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants