Skip to content

refactor(typing): use PEP 604 syntax for Optional annotations (UP045) - #8399

Merged
leandrodamascena merged 3 commits into
aws-powertools:developfrom
manshahH:refactor/pyupgrade-up045
Aug 28, 2026
Merged

refactor(typing): use PEP 604 syntax for Optional annotations (UP045)#8399
leandrodamascena merged 3 commits into
aws-powertools:developfrom
manshahH:refactor/pyupgrade-up045

Conversation

@manshahH

@manshahH manshahH commented Aug 24, 2026

Copy link
Copy Markdown
Contributor

Issue number: closes #8398

Summary

Clears the first of the four temporary pyupgrade ignores in ruff.toml, following the one rule per PR plan @leandrodamascena outlined in #8047.

Optional[X] becomes X | None across the repository, and "UP045" is removed from lint.ignore so the rule is enforced from here on.

Changes

  • 285 violations across 65 files: 24 in aws_lambda_powertools, 25 in examples, 15 in tests, plus ruff.toml
  • 283 fixed by the ruff autofix
  • 2 converted by hand
  • 63 Optional imports left unused by the conversion were removed, since F401 flags them

The two manual cases. Both are in aws_lambda_powertools/utilities/batch/types.py, where Optional[...] sits in a runtime assignment rather than an annotation:

BatchTypeModels = Optional[Union[...]]
BatchSqsTypeModel = Optional[Type[SqsRecordModel]]

Ruff correctly refuses to autofix these, because the expression is evaluated at import time and a rewrite changes the object that actually gets built. I converted them by hand and checked the result against the previous definitions:

Optional[Union[...]] == BatchTypeModels             -> True
Optional[Type[SqsRecordModel]] == BatchSqsTypeModel -> True

Union is deliberately left in place there, since it belongs to UP007. Removing it here would blur the one rule per PR split.

Docstrings. Twenty parameter descriptions and section comments inside files this PR already touches still referred to Optional[...] while the code below them had changed, so they were brought in line. Files not otherwise modified by this PR were left alone.

User experience

No user facing change. This is internal typing syntax only, and the runtime behaviour of every converted annotation and alias is unchanged.

For contributors, UP045 is now enforced, so the older Optional[X] spelling gets caught by make lint instead of being silently accepted.

Verification

  • ruff check passes with UP045 enforced
  • ruff format --check: 1310 files already formatted
  • Full suite excluding e2e: 2497 passed. Six failures are pre existing Windows only path and formatting issues that reproduce identically on a clean develop
  • mypy aws_lambda_powertools examples: same error count before and after, no new errors introduced
  • bandit security baseline and xenon complexity baseline both unaffected

Happy to carry on with UP007 and then UP035 in the same shape once this one looks right.

Acknowledgment


By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Convert Optional[X] to X | None across the codebase and remove the
temporary UP045 ignore from ruff.toml, so the fix and the removal of the
ignore land together.

283 of the 285 violations were handled by the ruff autofix. The two
remaining cases in utilities/batch/types.py are runtime assignments
rather than annotations, so they were converted by hand and verified to
compare equal to their previous definitions.

Union is left in place, since it belongs to UP007.
@manshahH
manshahH requested a review from a team as a code owner August 24, 2026 12:30
@manshahH
manshahH requested a review from svozza August 24, 2026 12:30
@boring-cyborg

boring-cyborg Bot commented Aug 24, 2026

Copy link
Copy Markdown

Thanks a lot for your first contribution! Please check out our contributing guidelines and don't hesitate to ask whatever you need.
In the meantime, check out the #python channel on our Powertools for AWS Lambda Discord: Invite link

@boring-cyborg boring-cyborg Bot added the tests label Aug 24, 2026
@powertools-for-aws-oss-automation powertools-for-aws-oss-automation Bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Aug 24, 2026
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.64%. Comparing base (a8df37d) to head (db523a3).

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #8399   +/-   ##
========================================
  Coverage    96.64%   96.64%           
========================================
  Files          296      296           
  Lines        14767    14767           
  Branches      1246     1246           
========================================
  Hits         14271    14271           
  Misses         361      361           
  Partials       135      135           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@leandrodamascena leandrodamascena left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for tackling this. I like this direction because it resolves the Ruff issue instead of adding another exception we would need to maintain as technical debt.

I found two small typos that currently fail at import time:

  • tests/e2e/utils/data_fetcher/logs.py:57
  • tests/e2e/utils/data_fetcher/traces.py:109

Could you please change any | None to:

Any | None

Lowercase any is Python's built-in function, so the current annotation raises a TypeError.

…nnotations

Optional[any] was tolerated because typing never validated its argument, so
the lowercase any went unnoticed. Once converted to any | None the expression
is evaluated at import time, and neither module has the future annotations
import, so importing them raised TypeError.

Also updates the matching docstring line in logs.py.

These modules are never imported by the test suite, since make test runs with
--ignore tests/e2e, which is why this was not caught locally.
@manshahH

Copy link
Copy Markdown
Contributor Author

Good catch, thank you. Fixed in 06b30b6.

Your diagnosis is right. Optional[any] was tolerated because typing never validates its argument, so the lowercase any sat there unnoticed. Once it became any | None the expression is evaluated at import time, and neither module has from __future__ import annotations, so importing them raised:

TypeError: unsupported operand type(s) for |: 'builtin_function_or_method' and 'NoneType'

Both signatures are now Any | None, with Any added to the typing import in logs.py. I also updated the matching docstring line just below the first one, at logs.py:64, which carried the same lowercase any.

This slipped past me because make test runs with --ignore tests/e2e, so these two modules are never imported by the suite. To be sure nothing else of the same shape was hiding in the diff, I walked the AST of all 65 changed files and checked every converted annotation for a left operand that is a builtin function rather than a type. 275 annotations checked, and those were the only two.

All four data_fetcher modules now import cleanly, and ruff check, ruff format --check and the test suite are unchanged.

@leandrodamascena leandrodamascena left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for fixing this and for checking the remaining converted annotations as well. I pulled the latest commit and confirmed that both modules now evaluate the annotations correctly, with Ruff checks also passing.

This fully addresses my review and resolves the Ruff rule without leaving another exception for us to maintain.

Thank you very much for the work. APPROVED!

@mergify

mergify Bot commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Tick the box to add this pull request to the merge queue (same as @mergifyio queue).

  • Queue this pull request

@sonarqubecloud

Copy link
Copy Markdown

@leandrodamascena
leandrodamascena merged commit 78fdd32 into aws-powertools:develop Aug 28, 2026
16 checks passed
@boring-cyborg

boring-cyborg Bot commented Aug 28, 2026

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!

@powertools-for-aws-oss-automation

Copy link
Copy Markdown

Awesome work, congrats on your first merged pull request and thank you for helping improve everyone's experience!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. tests

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Maintenance: Fix UP045 ruff lint violations (Optional[X] to X | None)

2 participants