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

No correspondence between string and int level format #209

Closed
Vedrillan opened this issue Feb 3, 2020 · 12 comments
Closed

No correspondence between string and int level format #209

Vedrillan opened this issue Feb 3, 2020 · 12 comments
Labels
enhancement Improvement to an already existing feature

Comments

@Vedrillan
Copy link

The behaviors when logging with the level set as "INFO" or 20 is different, though they share the same level number.

image

I noticed that because I wanted to use logging levels constant:

from logging import INFO
from loguru import logger
logger.log(INFO, "toto")

If the level number is registered in loguru it should be matched and used instead of displaying the generic "Level %d" % level_no message without any colorization.

It would also be nice to have Loguru providing the level numbers as constant (e.g loguru.INFO), otherwise levels are just magic strings that cannot be easily looked up or referenced.

@Delgan
Copy link
Owner

Delgan commented Feb 3, 2020

Hi @Vedrillan.

The way the levels work is indeed slightly different between logging and loguru. Using loguru, the he severity level is not mapped against any level name, bu this is made on purpose.

A level is basically a namedtuple:

Level(name='INFO', no=20, color='<bold>', icon='ℹ️')

I think what should be used to identify a level is its name, not its number. It's perfectly valid to create a custom level while another one exist with the same no, this shall not create any conflict (which would be the case if the no was also mapped to the level).

Adding constants to reference levels has been discussed, but it doesn't seem much useful to having to type loguru.INFO instead of "INFO". I don't think of them as being magic strings, it's just a level name and could be any other string. Moreover, the levels are not truly constants because they can actually be modified. Finally, I also like that Loguru's features are fully contained in from loguru import logger.

@Vedrillan
Copy link
Author

After more reading of the code, I indeed see that the levels are identified by their names.

This is a strange thing to me, as in my opinion log levels should be identified by both their names and levels. Changing the level or name of a "core" level, such as "INFO", would not make sens as it goes against common logging practices (of things I have used, ruby, php, javascript, syslog). Such changes would just generate confusion.

One could consider that developers are free to make "DEBUG" logs appear more critical that "EMERGENCY" but others prefer to capitalize on standards and common practices. I am obviously of second group, and if you are not that is alright, we all have our opinions and they do not have to be similar.

In the end I am only hoping for the existing levels to be constant, by their names and levels, across all my projects, to avoid the mess and pain that changing the "standardized level" would generate, but if it is not the philosophy of loguru then be it, I will just have to use something else that fits my requirements.

@Delgan
Copy link
Owner

Delgan commented Feb 4, 2020

So, are you suggesting to actually make the no attribute of each level immutable? That is something I could definitely consider. I totally agree that it doesn't make much sense to modify a level severity, I can't think of any use case for that and it would just cause surprising behaviour.

That is, logging with "INFO" will be guaranteed to always log with a severity of 20.

@Vedrillan
Copy link
Author

In my opinion both no and name of the core levels should be immutable.

The fact that the no is not linked to the name appeared to me as a bug because I thought it was the case, and as such using the no or name should have been displayed the same way.

If someone can change either one of the name or no, it would allow swapping levels severity and create a situation, that we both agree, does not make much sens and would probably only cause confusion.

@Delgan
Copy link
Owner

Delgan commented Feb 5, 2020

The name is already immutable in the sense that you can't remove nor modify it once a level has been added.

However, I'm still convinced that a no should not be linked to a name.
One name can only be mapped to one no, but one no can be used by multiple name.
I see no reason to fundamentally prevent user to create a custom "FANCY" level with a severity of 20, and I don't want to run in conflicts if two libraries decide to create their own level with a no of 42 for both.

@Delgan Delgan added the enhancement Improvement to an already existing feature label Feb 10, 2020
@Vedrillan
Copy link
Author

The no is a level filtering system, not a log type filtering one, so in my opinion for at least "core" levels it should not be possible to create a new level at the same no, if a level already exists you should just use it.

As you said this can not be applied to custom levels otherwise it would lead to issues if several lib created their own levels and end up with a no collision.

The more the conversation goes the more I understand why the logging system I used do not allow custom levels, to prevents special cases and fragile situations.

My opinion is to make core levels name and no immutable, which would link them for core levels, and disallow custom levels with same no as a core level. But that would be a big change for loguru as it gives away a good part of its customization capability.

@Delgan
Copy link
Owner

Delgan commented Feb 11, 2020

I do agree that core levels are more than enough, but some people like to use custom levels, so I tried to provide a working solution for their use case. You are talking about preventing special cases: that's one of the main reason I would prefer to avoid implementing any specific behavior for core levels.

From my point of view, it's more important to keep the API as simple as possible rather than allowing severity no to be mapped to level. It's easier to say "levels are only identifiable by their name" rather than to explain "levels are identifiable by their name, core levels are identifiable by no too, custom levels can't use core levels no".

To be honest, I still don't understand what bothers you about custom level using "core" severity, and I also don't understand why you think it's so important that levels be identifiable by their severity no. I think level=30 is less readable as you have to know that the "maximum" level is 50, while level="WARNING" is self-explanatory.

I'm pretty satisfied with the current levels API, I think it is a good compromise that is compatible with most use case while also maintaining some consistency. However, I understand that I cannot meet everyone's needs. I wish Loguru was compatible with your concerns, but for the reasons we discussed it does not seem fully possible. One thing for sure, though, is that I will made no immutable.

@Vedrillan
Copy link
Author

Using a string as a level identifier is actually not as self-explanatory as you say. You can not deduce the levels order by only their names. It is however possible with numerical values, of course as long as you know if the order is ascending or descending. Also I agree that level=40 is not the most readable, which is why

As of for why I am bothered by custom levels having the same height as core ones, it simply is against what the level purpose. The goal of the log level is to filter based on the desired height, so having several levels at the same height have no advantage, logging with a level of INFO or CUSTOM_LEVEL20 will have the same effect, which make usage of custom levels useless in that regards.

And now about why I think the name of core levels should be immutable as well, it is because changing them would break the interface contract of the logger. This would make the shortcuts like logger.debug(), logger.info() etc. not working anymore if the log is identified by its name or plain confusing if the level is identified by its height because logger.debug() will display a different level than DEBUG.

I hope this help you understand my point of view, it becomes hard to stay concise :p

@Delgan
Copy link
Owner

Delgan commented Feb 11, 2020

Yes, it helps, thank you for taking the time to write this. However, I do understand "why [you] think the name of core levels should be immutable", I don't understand why you think logger.log(20, "hello") should be strictly equivalent to logger.log("INFO", "hello") (I understand why it could be equivalent, but not why it needs to be).

Fundamentally, I probably agree with what you're saying, but in my opinion it's not worth it considering the drawbacks.

@Vedrillan
Copy link
Author

The biggest advantage I see is that it would make logging and loguru pretty much interchangeable, it would be possible to migrate from one to another transparently, which would be very nice.

@Delgan
Copy link
Owner

Delgan commented Feb 14, 2020

Ok, thanks. It would somehow ease the migration process, but it does not seem very useful in the context of Loguru solely hence I prefer to keep the API as is.

@Delgan
Copy link
Owner

Delgan commented Apr 8, 2020

I made the level no immutable, it will be effective in the next version. ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improvement to an already existing feature
Projects
None yet
Development

No branches or pull requests

2 participants