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

Add types #282

Merged
merged 23 commits into from
Nov 12, 2020
Merged

Add types #282

merged 23 commits into from
Nov 12, 2020

Conversation

hynek
Copy link
Owner

@hynek hynek commented Nov 10, 2020

So far I've tested it with a bunch of my own projects and it seems to be working great. Not sure if I'm missing something?

The big concession we've had to make is adding a few wrapper methods/functions that only add correct types:

  • structlog.stdlib.get_logger()
  • structlog.stdlib.BoundLogger.bind() (and the whole rest)

I think that's tolerable from a performance stance.

@hynek hynek mentioned this pull request Nov 10, 2020
@hynek
Copy link
Owner Author

hynek commented Nov 10, 2020

I've added a chapter with some thoughts: https://structlog--282.org.readthedocs.build/en/282/types.html

And a quick workaround if it breaks your CI.

MANIFEST.in Outdated Show resolved Hide resolved
docs/types.rst Outdated Show resolved Hide resolved
docs/types.rst Outdated Show resolved Hide resolved
docs/types.rst Show resolved Hide resolved
docs/standard-library.rst Show resolved Hide resolved
hynek and others added 3 commits November 10, 2020 16:04
Co-authored-by: Éric Araujo <merwok@netwok.org>
Co-authored-by: Éric Araujo <merwok@netwok.org>
It seems to work without but who the hell even knows.
Copy link

@asottile asottile left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I time-boxed myself 15 minutes to look at this, so sorry for the incomplete review -- hope some of this is helpful <3 a typing nerd

additionally, you might want to check out flake8-typing-imports:

$ flake8 --min-python-version 3.6.0 src/
src/structlog/testing.py:14:1: TYP001 guard import by `if TYPE_CHECKING:`: NoReturn (not in 3.6.0, 3.6.1)

(notably, NoReturn doesn't exist in 3.6.0 and 3.6.1 but those are compatible with your python_requires)

pyproject.toml Show resolved Hide resolved
src/structlog/_base.py Show resolved Hide resolved
src/structlog/_base.py Show resolved Hide resolved
src/structlog/_base.py Outdated Show resolved Hide resolved
src/structlog/_base.py Show resolved Hide resolved
src/structlog/_frames.py Show resolved Hide resolved
@hynek
Copy link
Owner Author

hynek commented Nov 10, 2020

I time-boxed myself 15 minutes to look at this

Thank you so much! ❤️

NoReturn

This shouldn't be a problem because I depend on typing-extensions on Python < 3.8! I need it for Protocol

@asottile
Copy link

I time-boxed myself 15 minutes to look at this

Thank you so much! ❤️

NoReturn

This shouldn't be a problem because I depend on typing-extensions on Python < 3.8! I need it for Protocol

you can cheese protocol with (example):

if TYPE_CHECKING:
    from typing import Protocol
else:
    Protocol = object

though if you're already committed to typing-extensions as a dependency then nbd!

src/structlog/_config.py Outdated Show resolved Hide resolved
src/structlog/stdlib.py Outdated Show resolved Hide resolved
@hynek
Copy link
Owner Author

hynek commented Nov 11, 2020

For everyone following along: I've noticed that the type hints of structlog.get_logger() were bogus: BindableLogger is useless if you want to log something and everybody would have to cast. So now it's Any. More on that in the updated https://structlog--282.org.readthedocs.build/en/282/types.html (I hope it will keep updating today…)

Copy link
Contributor

@Zac-HD Zac-HD left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks great 😁

CHANGELOG.rst Outdated Show resolved Hide resolved
docs/api.rst Outdated Show resolved Hide resolved
Co-authored-by: Zac Hatfield-Dodds <Zac-HD@users.noreply.github.com>
hynek and others added 4 commits November 12, 2020 06:45
Co-authored-by: Zac Hatfield-Dodds <Zac-HD@users.noreply.github.com>
@hynek hynek merged commit a654599 into master Nov 12, 2020
@hynek hynek deleted the types branch November 12, 2020 10:23
@hynek
Copy link
Owner Author

hynek commented Nov 12, 2020

Thanks everyone! If something else comes up, feel free to open new issues.

jkglasbrenner added a commit to usnistgov/dioptra that referenced this pull request Jun 11, 2021
The structlog 20.2.0 update added proper type annotations to the package, see
hynek/structlog#223, hynek/structlog#282. Updated
package, docker, and dev configuration files to ensure structlog 20.2.0 or higher is always
installed. Updated `mitre.securingai` source and unit testing code so that all `get_logger()` calls
use the `structlog.stdlib` import path and also replaced the `BoundLoggerLazyProxy` type annotation
with `structlog.stdlib.BoundLogger` as suggested in the 20.2.0 documentation, see
https://www.structlog.org/en/20.2.0/types.html.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants