-
Notifications
You must be signed in to change notification settings - Fork 705
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
Improve typing of PropagateHandler and InterceptHandler #971
Conversation
# Get corresponding Loguru level if it exists. | ||
level: str | int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The typehint can actually be put on the first declaration as it's the first declaration that is being used to determine the type for the same named variable. This works for Mypy, Pyright and Pyre.
try:
level: int | str = logger.level(record.levelname).name
except ValueError:
level = record.levelno
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@quantumpacket Is it considered best practice? I felt it might be confusing for the reader, so I decided to explicitly declare the level
types before the assignment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what would consider it best practice or not, it's allowed by all type checkers listed. I guess it's a matter of preference at this point. 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I'll leave it as is then, but thanks for the heads-up.
Was up all night reading about frames/stacks and I came up with another way to get the depth = 1
for frame in traceback.extract_stack():
if frame.filename == logging.__file__:
depth += 1 Python, a thousand and one ways to do the same thing. 😆 |
Haha, nice find! This implementation indeed looks pretty good, at the cost of a bit more memory usage. I personally prefer to deal directly with the |
@Delgan if you still want to use frame, depth = inspect.currentframe(), 1
while frame:
if inspect.getfile(frame) == logging.__file__:
depth += 1
frame = frame.f_back |
I don't really have any particular preference. I've just documented a solution that solves the problems you mentioned, so I didn't dwell on it too much since I'm planning to replace these classes anyway. But thanks for sharing. |
See #654 (comment) and #449 (comment).