sentry-python icon indicating copy to clipboard operation
sentry-python copied to clipboard

Convert sentry_sdk type annotations into the modern format

Open zsol opened this issue 1 week ago • 10 comments

Description

This PR converts all Python 2-style type annotations (in comments) into Python 3-style annotations (after : tokens, and inside the AST).

There's a large amount of changes in here, apologies for that. I generated these changes using:

  1. A LibCST codemod to do the bulk of the work: uv run --with libcst python -m libcst.tool codemod convert_type_comments.ConvertTypeComments --no-format -j1 sentry_sdk
  2. Some manual fixups here and there (see below)
  3. Run com2ann to fix the rest: uvx com2ann sentry_sdk
  4. Reformat: uvx ruff format sentry_sdk

Manual changes

  • The LibCST command made a bunch of silly mistakes like turning foo = None # type: Foo # noqa into foo: "Foo # noqa" = None, which were trivial to fix
  • When turning foo = None # type: Foo into foo: Foo = None, mypy starts raising an error on the second form - I suppressed these using type: ignore[assignment]

Test plan

mypy should continue being green, as evidenced by running uvx tox -e linters

zsol avatar Dec 09 '25 17:12 zsol

Hi @zsol,

Thank you for the PR. Upgrading the type annotation style is a breaking change because we still support Python 3.6.

We will do this work on the major branch, and can keep you updated.

alexander-alderman-webb avatar Dec 11 '25 16:12 alexander-alderman-webb

My understanding is that PEP 526 is compatible with Python 3.6 (but not Python 3.5), so IIUC this shouldn't break any Python 3.6 users.

charliermarsh avatar Dec 11 '25 17:12 charliermarsh

You're correct that using PEP 526 is possible on Python 3.6.

The reason we didn't do the type transition yet is that we use some newer types, such as typing.Literal and typing.TypedDict.

We currently type check on newer Python versions, while supporting 3.6 at runtime for users. This is possible because old-style type annotations together with typing.TYPE_CHECKING prevent user runtimes from having to import types.

You can achieve an analogous result on Python 3.7 with new-style type annotations, but I believe you require __future__.annotations, which is unfortunately only available in Python 3.7. See https://peps.python.org/pep-0563/.

alexander-alderman-webb avatar Dec 12 '25 12:12 alexander-alderman-webb

if only Literal and TypedDict are the blockers, I'm fine with removing those since I massively dislike TypedDict anyway

sl0thentr0py avatar Dec 12 '25 12:12 sl0thentr0py

It's mainly those two. There's a few instances of other new types like typing.NotRequired and typing_extensions.Unpack.

alexander-alderman-webb avatar Dec 12 '25 12:12 alexander-alderman-webb

You can achieve an analogous result on Python 3.7 with new-style type annotations, but I believe you require __future__.annotations, which is unfortunately only available in Python 3.7. See peps.python.org/pep-0563.

You can do the same on Python 3.6 without __future__.annotations if you're happy to use stringized/quoted annotations. Which this PR does.

AlexWaygood avatar Dec 12 '25 12:12 AlexWaygood

Great to know! CI seems to fail because there are some places in which the annotations are not in quotes in this PR.

What is the motivation for the PR? We need to transition regardless, but for us to prioritize bringing this over the line it would be helpful to understand if there are limitations beyond comment-based types becoming less idiomatic. Are there new tools for which the comment-based annotations do not work?

alexander-alderman-webb avatar Dec 12 '25 13:12 alexander-alderman-webb

Great to know! CI seems to fail because there are some places in which the annotations are not in quotes in this PR.

Ah, cool, that's a bug in the PR! Thanks for the heads-up (cc. @zsol)

What is the motivation for the PR? We need to transition regardless, but for us to prioritize bringing this over the line it would be helpful to understand if there are limitations beyond comment-based types becoming less idiomatic. Are there new tools for which the comment-based annotations do not work?

Yes. The main motivation is that newer type checkers such as ty do not support the legacy syntax. (Full disclosure: I'm a ty developer and Zsolt's a colleague, though he doesn't work on ty.) It would be quite a bit of work for us to add support for the legacy syntax to ty, so we're reluctant to do so given that it's only required for compatibility with Python <=3.5. Meanwhile, we're big fans of Sentry, and have other code elsewhere that uses Sentry. We'd love to type-check that code with ty, but doing so currently produces poor results, because ty can't understand any of Sentry's legacy type hints.

Note that there have also been discussions about deprecating the legacy syntax from the type system altogether, albeit those discussions have been dormant for a while now:

  • https://mail.python.org/archives/list/[email protected]/thread/66JDHQ2I3U3CPUIYA43W7SPEJLLPUETG/
  • https://github.com/python/mypy/issues/12947

Other tools such as pyflakes/flake8 dropped support for type comments a while back, and Ruff has never supported them.

AlexWaygood avatar Dec 12 '25 13:12 AlexWaygood

Thank you for the detailed explanation, and thank you for using Sentry!

We didn't transition yet because of internal misunderstanding that we need __future__.annotations. We are happy to support your efforts so that others can use ty with sentry-python.

The steps already in the PR descriptions are great since we'll verify the manual fixes. If you have a systematic way of putting all annotations in quotes keep us updated. Otherwise, I'll pick up from here next week.

alexander-alderman-webb avatar Dec 12 '25 13:12 alexander-alderman-webb

I'm happy to get this PR green for you!

zsol avatar Dec 12 '25 14:12 zsol

I believe I've resolved all the unquoted type annotations and pulled in changes from master. Let's see what CI says

zsol avatar Dec 15 '25 12:12 zsol

Great, thanks for making CI green!

I'll run some diffs and then merge if everything looks good.

alexander-alderman-webb avatar Dec 15 '25 12:12 alexander-alderman-webb

Codecov Report

:x: Patch coverage is 98.83659% with 22 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 84.24%. Comparing base (2ce4379) to head (a7313ae). :warning: Report is 16 commits behind head on master.

Files with missing lines Patch % Lines
sentry_sdk/integrations/aws_lambda.py 60.00% 8 Missing :warning:
sentry_sdk/integrations/gcp.py 71.42% 2 Missing :warning:
sentry_sdk/integrations/google_genai/__init__.py 88.88% 2 Missing :warning:
sentry_sdk/integrations/spark/spark_driver.py 94.59% 2 Missing :warning:
sentry_sdk/integrations/asgi.py 90.90% 1 Missing :warning:
sentry_sdk/integrations/atexit.py 80.00% 1 Missing :warning:
sentry_sdk/integrations/boto3.py 88.88% 1 Missing :warning:
sentry_sdk/integrations/django/__init__.py 97.36% 1 Missing :warning:
sentry_sdk/integrations/django/asgi.py 93.75% 1 Missing :warning:
sentry_sdk/integrations/langchain.py 98.07% 1 Missing :warning:
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #5206      +/-   ##
==========================================
- Coverage   84.25%   84.24%   -0.02%     
==========================================
  Files         181      182       +1     
  Lines       18463    18550      +87     
  Branches     3288     3299      +11     
==========================================
+ Hits        15556    15627      +71     
- Misses       1894     1911      +17     
+ Partials     1013     1012       -1     
Files with missing lines Coverage Δ
sentry_sdk/_compat.py 94.87% <100.00%> (ø)
sentry_sdk/_init_implementation.py 95.83% <100.00%> (ø)
sentry_sdk/_log_batcher.py 80.89% <100.00%> (ø)
sentry_sdk/_lru_cache.py 100.00% <100.00%> (ø)
sentry_sdk/_metrics_batcher.py 73.86% <100.00%> (ø)
sentry_sdk/_queue.py 62.22% <100.00%> (ø)
sentry_sdk/_werkzeug.py 48.14% <100.00%> (ø)
sentry_sdk/ai/monitoring.py 85.33% <100.00%> (ø)
sentry_sdk/ai/utils.py 94.68% <100.00%> (ø)
sentry_sdk/api.py 86.57% <100.00%> (ø)
... and 153 more

codecov[bot] avatar Dec 15 '25 13:12 codecov[bot]

Thanks for the quick turnaround @alexander-alderman-webb!

zsol avatar Dec 15 '25 13:12 zsol