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

False positive on extra kwargs #93

Open
kornicameister opened this issue Feb 9, 2021 · 3 comments
Open

False positive on extra kwargs #93

kornicameister opened this issue Feb 9, 2021 · 3 comments
Labels
question Further information is requested

Comments

@kornicameister
Copy link
Owner

kornicameister commented Feb 9, 2021

Consider following piece of code:

logger.info(
    'Booting up worker',
    extra={'queue': queue_name},
)

It is obviously flagged as:

[mypy] Not all arguments converted during string formatting  [str-format] [E] 
@kornicameister kornicameister changed the title False positive on extra kwargsstory False positive on extra kwargs Feb 9, 2021
@kornicameister
Copy link
Owner Author

@Delgan @ThibaultLemaire I'd like to gather your opinion on that please ;-)
Would it be consider a good call or a false positive ?

@kornicameister kornicameister added the question Further information is requested label Feb 9, 2021
@ThibaultLemaire
Copy link
Contributor

ThibaultLemaire commented Feb 9, 2021

It doesn't do what it seems, so I'd say it's good that it raises an error. The extra dict is intended to be passed by **kwargs, so this won't work with a sink configured to use extra[queue]. (Although the error doesn't exactly tell you all that)

The real problem though is that it will tell you the same for

logger.info(
    'Booting up worker',
    queue=queue_name,
)

But I think I've already touched on that somewhere that I can't find again at this time.

@Delgan
Copy link
Collaborator

Delgan commented Feb 9, 2021

Technically it's correct code. Basically, I would never consider this a error within Loguru.
However, it may be interesting to warn Mypy users about possible misuse. Most of the time, it's indeed a mistake. Especially for people migrating from standard logging. As long as it's possible to disable the warning for the theoretically valid use case, it seems to be a good catch in general.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants