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

regression in new logging-format-style scheme #3361

Closed
belm0 opened this issue Jan 24, 2020 · 5 comments · Fixed by #3475
Closed

regression in new logging-format-style scheme #3361

belm0 opened this issue Jan 24, 2020 · 5 comments · Fixed by #3475
Assignees
Labels
Blocker 🙅 Blocks the next release
Milestone

Comments

@belm0
Copy link
Contributor

belm0 commented Jan 24, 2020

I have a project which uses both % and f-string styles for logging

Previously lint could pass using logging-format-style=old in combination with disable=logging-fstring-interpolation

Now, logging-fstring-interpolation is retired, and logging-format-style allows choice of only one style. So I'm forced to disable logging-format-interpolation completely.

(Reference: #2395, #3095)

$ pylint --version
pylint 2.4.4
astroid 2.3.3
Python 3.7.5 (default, Nov  1 2019, 02:16:38)
[Clang 10.0.0 (clang-1000.11.45.5)]
@AWhetter AWhetter added the Blocker 🙅 Blocks the next release label Jan 28, 2020
@AWhetter AWhetter self-assigned this Jan 28, 2020
@AWhetter
Copy link
Contributor

Originally I had only allowed a single option to be chosen so that the message could give better guidance on which types of logging formatting were invalid. But it's more complicated than that. Originally we were relying on the user to tell us which type of formatting was being used so that we knew how to interpret the arguments. We should be able to figure this out ourselves though. We can reliably detect whether a format string or the new style format function is being called, and by process of elimination whether it's old style formatting.

You're right that it's not ideal that only one style of formatting is allowed. The message might have to be more vague about how to solve the message. We could possibly base it on which of the messages are enabled or disabled. I don't know if it makes sense to keep the logging-format-style option because really that doesn't dictate anything if we can figure out which style of formatting is in use per call, and so it only has the potential to get out of sync with which messages are enabled an disabled.

So to summarise, I am proposing:

  • We deprecate the logging-format-style option.
  • We bring back logging-fstring-interpolation.
  • We add logging-old-style-interpolation.

@PCManticore What do you think? Is the trade off of a more vague message worth the additional flexibility?

I've marked this as a blocker because if we decide that what #3095 implemented isn't right, then we need to rectify that before we do a release with logging-fstring-interpolation having been deprecated.

@PCManticore
Copy link
Contributor

@AWhetter Thanks for the clear proposal, I'm on board with it. Let's get this in time for 2.5.

@PCManticore PCManticore added this to the Next minor release milestone Jan 30, 2020
@belm0
Copy link
Contributor Author

belm0 commented Jan 30, 2020

This will address #3362 as well?

@AWhetter
Copy link
Contributor

It will, yes.

@vfilimonov
Copy link

Hello

At which version was W1203 (logging-fstring-interpolation) removed?

I have disabled it (per-file or per-line) in many parts of different projects and now I have complaints about wrong option (W1203) and (logging-fstring-interpolation) all over the place after update to 2.4.4.

While 2.5 with the final conclusion on the logging is not released yet, I'd like to roll back rather than replacing W1203 with W1202 in each file.

Thanks!

@PCManticore PCManticore unpinned this issue Apr 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker 🙅 Blocks the next release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants