Add some missing logging features by spagh-eddie · Pull Request #5388 · python/typeshed

spagh-eddie · 2021-05-09T22:35:51Z

This handles #5257, #5015, and changes two type to Type[...] for #343

~~Note I annotated validation_pattern, IDENTIFIER, and extMatch as Any since I cannot do this, which would make a variable, not a type alias.~~

if sys.version_info >= (3, 7):
    import re
    _Pattern = re.Pattern
else:
    import regex
    _Pattern = regex.Regex
IDENTIFIER: _Pattern

Please note that SMTPHandler.password is not always created depending on what is passed in! But since I know of no workarounds for maybe-exsiting-attributes and the documents suggest:

we usually prefer false negatives (no errors for wrong code) over false positives (type errors for correct code)

I have added it password: str.

I am not fixing everything I see in this PR, I have left a lot of logging.config as-is

$ stubtest logging --custom-typeshed-dir . --concise
logging.config.BaseConfigurator is not present in stub
logging.config.ConvertingDict is not present in stub
logging.config.ConvertingList is not present in stub
logging.config.ConvertingMixin is not present in stub
logging.config.ConvertingTuple is not present in stub
logging.config.DictConfigurator is not present in stub
logging.config.dictConfigClass is not present in stub

github-actions · 2021-05-10T01:49:14Z

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

srittau

Thank you, that's very thorough! A few minor notes below about the things you mentioned.

wiseaidev · 2021-05-10T17:56:08Z

+    def usesTime(self) -> bool: ...  # undocumented
+
+class BufferingFormatter:
+    linefmt: Formatter


I think line format could be a string like: '%(something)s %(something)s'. So, linefmt should be changed to:

linefmt: Union[str, Formatter]

Technically it can be anything with a .format() method, even a string. But that .format() method will be called with only one argument, a log record object. It is possible to create a string that formats them in a useful way, such as "{0.name}: {0.msg}", although that doesn't seem to be how it's intended to be used.

So the correct fix would be a protocol? We could leave as Any for the moment, though.

Honestly I would just use Formatter. It's what the author of the code intended. (This is old code, likely written before str.format was a thing)

I was under the same impression as @Akuli, intended to be a Formatter

wiseaidev · 2021-05-10T18:11:34Z

+
+class BufferingFormatter:
+    linefmt: Formatter
+    def __init__(self, linefmt: Optional[Formatter] = ...) -> None: ...


Same here:

def __init__(self, linefmt: Optional[Union[str, Formatter]] = ...) -> None: ...

- python#5388 (comment)

srittau

If someone want to change Formatter to a protocol, they can do it in a separate PR. I'm fine with it being a concrete class for now.

spaghEddieDoyle added 2 commits May 9, 2021 15:35

Add some missing features

9be69c7

Add QueueListener.handle

227fc20

This comment has been minimized.

Sign in to view

This was changed in 3.10

4338f48

spagh-eddie marked this pull request as draft May 10, 2021 01:21

Remove duped extra

3e7ff43

spagh-eddie marked this pull request as ready for review May 10, 2021 02:04

srittau reviewed May 10, 2021

View reviewed changes

Comment thread stdlib/logging/__init__.pyi Outdated

Comment thread stdlib/logging/config.pyi Outdated

Comment thread stdlib/logging/handlers.pyi Outdated

Comment thread stdlib/logging/handlers.pyi

wiseaidev reviewed May 10, 2021

View reviewed changes

spaghEddieDoyle added 3 commits May 10, 2021 23:23

validation_pattern was added 3.8

5e9f505

- python#5388 (comment)

Add note password is not always an attribute

f0fc5b2

- python#5388 (comment)

was unaware of typing.Pattern

88c1516

srittau approved these changes May 11, 2021

View reviewed changes

srittau merged commit 4c72f7f into python:master May 11, 2021

This was referenced May 12, 2021

Missing type annotations for properties on logging.LoggerAdapter #5015

Closed

"FileHandler" has no attribute "set_name" #5257

Closed

hauntsaninja mentioned this pull request Nov 28, 2021

stubtest: should unused allowlist entries be an error in stdlib? #6406

Closed

Akuli mentioned this pull request Apr 24, 2023

Simplify logging.config type imports #10077

Merged

Conversation

spagh-eddie commented May 9, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

github-actions Bot commented May 10, 2021

Uh oh!

srittau left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wiseaidev May 10, 2021

Choose a reason for hiding this comment

Uh oh!

Akuli May 10, 2021

Choose a reason for hiding this comment

Uh oh!

wiseaidev May 10, 2021

Choose a reason for hiding this comment

Uh oh!

srittau May 10, 2021

Choose a reason for hiding this comment

Uh oh!

Akuli May 10, 2021

Choose a reason for hiding this comment

Uh oh!

spagh-eddie May 11, 2021

Choose a reason for hiding this comment

Uh oh!

wiseaidev May 10, 2021

Choose a reason for hiding this comment

Uh oh!

srittau left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

spagh-eddie commented May 9, 2021 •

edited

Loading