Flesh out the few remaining incomplete annotations

[layday]

_Logger.add's sink parameter.can accept
all of the same types as _HandlerConfig.sink.
'extra' is Dict[str, Any] where in
reality the value is anything that can be
serialised and the key is the name of a keyword
argument.
parse overloads can be implemented without
@staticmethod since _Logger is not part of the
public API (the logger is a singleton).
Renamed Logger to _Logger since Logger
does not exist at runtime.

[Delgan]

Hey @layday, thanks for contributing to Loguru!

Let me clarify why type hints are implemented in this way.

_Logger.add's sink parameter.can accept
all of the same types as _HandlerConfig.sink.

The .add() method is "type hinted" thrice thanks to the @overload decorator. The purpose is to avoid the method to be called with invalid arguments depending on the sink's type used.

For example, logger.add("file.log", rotation="5 MB") is correct while logger.add(sys.stderr, rotation="5 MB") is not. We want mypy to warn about such misuse. That's why there exists both sink: Union[TextIO, Writable, Callable[[Message], None], Handler] and sink: Union[str, PathLike] signatures, with additional arguments that differ for the latter.

'extra' is Dict[str, Any] where in
reality the value is anything that can be
serialised and the key is the name of a keyword
argument.

I deliberately made extra type very permissive because it is in fact a basic dict that can include anything as a key too! Sure, calling .bind() will only assign a str to Any, but the extra dict is directly mutable though logger.configure(extra={123: 456}) or logger.patch(lambda r: r["extra"].update({123: 456})).

I don't expect this to be used very much in practice, but still it's possible and nothing forbids it. I don't think we should restrict the user on this point.

parse overloads can be implemented without
@staticmethod since _Logger is not part of the
public API (the logger is a singleton).

Clever. Would there be a problem if the signature between the type hint and the implementation are different (since there is no self)?

Renamed Logger to _Logger since Logger
does not exist at runtime.

The _Logger class is not accessible to discourage instantiating new logger object this way. However, the type should be public because it's perfectly legit to annotate a function which accepts a logger object:

from __future__ import annotations

def prepare_logger(logger: loguru.Logger):
    logger.configure(...)

[layday]

The .add() method is "type hinted" thrice thanks to the @overload decorator. The purpose is to avoid the method to be called with invalid arguments depending on the sink's type used.

Apologies, I missed that.

I don't expect this to be used very much in practice, but still it's possible and nothing forbids it. I don't think we should restrict the user on this point.

Thank you for clarifying. In that case, I would be inclined to annotate it with Dict[Any, Any] because a bare dict is (in some sense) underspecified and type checkers might complain that its parameters are unknown, which is the case in Pyright.

Clever. Would there be a problem if the signature between the type hint and the implementation are different (since there is no self)?

I don't think so - not unless a type checker were to attempt to infer the types from source, which, ahem, should never happen in the presence of a .pyi file.

The _Logger class is not accessible to discourage instantiating new logger object this way. However, the type should be public because it's perfectly legit to annotate a function which accepts a logger object ...

That does make sense but exposing a name that does not exist at runtime is rather unusual. Users would still be able to access the Logger class by calling type on logger, though I suppose this might be a little arcane:

from __future__ import annotations
from typing import TYPE_CHECKING

if TYPE_CHECKING:
    Logger = type(loguru.logger)

def prepare_logger(logger: Logger):
    logger.configure(...)

I'll revert it.

[layday]

Whoops, I pinged both @staticmethod and @overload in this PR ;P

[Delgan]

Thanks for the update!

One final observation: shouldn't parse() return a generator of Dict[str, Any] instead of Dict[str, str] / Dict[str, bytes]? The original aim of this function was to make it easier to convert values extracted from logs to their appropriate type.

I will fix it during the merge so you don't have to bother updating the PR once again.

That does make sense but exposing a name that does not exist at runtime is rather unusual.

Yes is it. 😄
It's the best compromise I've found. It suits me, even if it's a bit surprising. I think the documentation keeps that clear. Also there are other types which don't exist at runtime but are very useful for typing, such as the Record dict for example.

Whoops, I pinged both @staticmethod and @overload in this PR ;P

You can select the part of the message you want to quote and type r instead of copy/pasting it, it preserves formatting. ;)

[layday]

You can select the part of the message you want to quote and type r instead of copy/pasting it, it preserves formatting. ;)

Oh cool, I didn't know that - thanks!

One final observation: shouldn't parse() return a generator of Dict[str, Any] instead of Dict[str, str] / Dict[str, bytes]? The original aim of this function was to make it easier to convert values extracted from logs to their appropriate type.

You are correct - I think it was a find/replace that went wrong.

[Delgan]

Finally merged, thanks for your time @layday!