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

Can't chain .opt() calls #192

Closed
gazpachoking opened this issue Dec 23, 2019 · 12 comments
Closed

Can't chain .opt() calls #192

gazpachoking opened this issue Dec 23, 2019 · 12 comments
Labels
documentation Improvements or additions to documentation enhancement Improvement to an already existing feature wontfix This will not be worked on

Comments

@gazpachoking
Copy link
Contributor

Calling opt more than once on a logger instance overrides all options that aren't specified to default, meaning you can't chain .opt() calls together.
For example in the following call, the color codes are ignored.

logger.opt(ansi=True).opt(depth=1).debug('<red>aoeu</>')

This doesn't make a lot of sense when done directly like this, but if you are using the recipe for adding custom loglevel methods, it calls .opt() to set the depth. The consequence of this is that you can no longer use .opt() directly from you code, since it will always be overridden by the depth call, including all of the options other than depth.

@Delgan
Copy link
Owner

Delgan commented Dec 26, 2019

Huh, this is a serious design issue indeed. Thanks for the report and the pull request.

I'm not a big fan of sentinel values, as it makes the signature of the function less readable. I wonder if there is any other possible workaround... If not, I will probably merge your fix. 👍

@Delgan Delgan added the enhancement Improvement to an already existing feature label Dec 26, 2019
@gazpachoking
Copy link
Contributor Author

Could switch to **kwargs instead of having a sentinel value... but that makes the signature even worse. Not sure a better way. At least the signature in the pyi file is still nice with type hints.

@Delgan
Copy link
Owner

Delgan commented Dec 26, 2019

I don't think there's much point in chaining opt() calls, apart from the problem with custom levels you've raised. So, what do you think of replacing the documented recipe with:

from functools import partialmethod

logger.__class__.foobar = partialmethod(logger.__class__.log, "foobar")

I was delightfully surprised to find out that it worked just fine! I guess the Python interpreter somehow optimize partialmethod() so it does not add another frame to the stack, thus avoiding the need for opt(depth=1). Then the foobar() logging method can be combined with any opt() attribute without problem.

@gazpachoking
Copy link
Contributor Author

I think that recipe change sounds like a good plan regardless of whether opt gets changed.
I sorta like the idea of it working as expected when chained, but it's much less important with that updated recipe. The only other instance I can think of is if someone perhaps wanted to do something like turn on colors by default when logging in only one of their modules. e.g.

from loguru import logger

logger = logger.opt(ansi=True)


try:
    somethingdangerous()
except:
    logger.opt(exception=True).debug('Everything <blue>went</> wrong')

@Delgan
Copy link
Owner

Delgan commented Dec 26, 2019

It seems possible to achieve the desired behavior using the same principle:

from functools import partial

logger = logger.opt(ansi=True)
logger.opt = partial(logger.opt, ansi=True)

Of course, this is not very intuitive, so this recipe needs to be properly documented. Nevertheless, I prefer this workaround. None of the solutions are perfect, so it's a matter of perspective, and I think it's more important to have a "clean" signature rather than allowing opt() chain calls (actually, see 2b8e6f9).

@gazpachoking
Copy link
Contributor Author

Sounds good, I'll defer to you on this one. Might also be worth a mention in the .opt docs that the last call is the only one that takes effect.

@Delgan
Copy link
Owner

Delgan commented Dec 27, 2019

I updated the recipe for custom level, created a new entry about opt(ansi=True) and added a note in the opt() documentation. Thanks again for the report. 😉

@Delgan Delgan closed this as completed Dec 27, 2019
@Delgan Delgan added documentation Improvements or additions to documentation wontfix This will not be worked on labels Dec 27, 2019
@gazpachoking
Copy link
Contributor Author

Awesome, looks good. Thanks!

@mifluder
Copy link

mifluder commented Jun 6, 2022

Do You have any way of working with logger.__class__.foobar = partialmethod(logger.__class__.log, "foobar") and mypy? Obviously mypy can't see that loguru.logger has attribute "foobar" and I didn't find any WA for this.

@Delgan
Copy link
Owner

Delgan commented Jun 6, 2022

@michalplat I think your problem relates to this mypy issue: python/mypy#5363

There is even a comment mentioning the exact same problem but using the standard logging library. Unfortunately, there does not seem to be an accepted solution. 😬

@mifluder
Copy link

mifluder commented Jun 6, 2022

Thanks for Your input 😄 I think I'll just use pure .log( in that situation. Maybe this is possible area of interests of @kornicameister and loguru-mypy plugin? I am not aware what are plugins used for but maybe for this 😁 If I come up with neat solution I'll update here but I don't have much hope for it😉

@kornicameister
Copy link
Contributor

I don't think I will be able to make any changes. Frankly I am quite busy nowadays. If you feel like you're up too, give it a shot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement Improvement to an already existing feature wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants