Skip to content

MAINT: Modernize Optional/Union typing to PEP 604 syntax#2109

Merged
romanlutz merged 3 commits into
microsoft:mainfrom
romanlutz:romanlutz-modernize-typing-syntax
Jul 2, 2026
Merged

MAINT: Modernize Optional/Union typing to PEP 604 syntax#2109
romanlutz merged 3 commits into
microsoft:mainfrom
romanlutz:romanlutz-modernize-typing-syntax

Conversation

@romanlutz

Copy link
Copy Markdown
Contributor

Description

Phase 1 of a typing-syntax cleanup. The codebase mixes legacy typing.Optional / typing.Union with the modern PEP 604 X | None / A | B syntax that our style guide requires (and that ruff's FA rule already enforces). This PR modernizes a first batch of 10 priority files so they are consistent with that convention. No behavior changes.

What changed:

  • Replaced Optional[T] with T | None and Union[A, B] with A | B in annotations and in docstring type references.
  • Added from __future__ import annotations to the 8 files whose annotations needed it, so the unions can be written unquoted (X | None) rather than as quoted forward-ref strings. This avoids the runtime TypeError you get from evaluating "X" | None when a referenced name is only imported under TYPE_CHECKING.
  • Moved annotation-only imports into TYPE_CHECKING blocks to satisfy the repo's flake8-type-checking (TCH) rules, keeping runtime-needed imports at module level. This matches the pattern already used by the ~35 existing future-annotations modules.
  • Two files (memory/storage/storage.py, memory/storage/serializers.py) only needed docstring type-reference updates, so they get no import or annotation changes.

Worth noting: two candidate files were intentionally left untouched because they use Union at runtime (backend/services/converter_service.py does origin is Union checks; scenario/core/attack_technique_factory.py does the same), so converting them would change behavior. Verified that the introspection consumers of these annotations (brick_contract.enforce_keyword_only_init, the converter-service catalog, and the factory's get_type_hints call) are unaffected by the switch to deferred annotations.

Tests and Documentation

No new tests are needed since this is a no-behavior-change typing refactor. Validated with the existing tooling:

  • ruff check and ruff format --check pass on all touched files.
  • ty check passes (confirms every unquoted forward-ref name resolves via its TYPE_CHECKING import).
  • Import-tested all affected modules plus the two intentionally-unchanged runtime-Union modules.
  • Ran the relevant unit suites (scorers, prompt converter, atomic attack, conversation manager, multi-prompt sending, tree of attacks, attack technique factory, converter service) after merging latest main: all passing.

No documentation (JupyText) changes are required for this change.

Copilot AI added 3 commits June 30, 2026 07:03
Replace Optional[T] with T | None and Union[A, B] with A | B across priority
files, drop now-unused Optional/Union imports, and update docstring type
references to the modern form. TYPE_CHECKING forward-ref unions use the
whole-string form ("X | None") so annotations stay valid at runtime on
Python 3.10+ (these modules have no `from __future__ import annotations`).

No behavior changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…notations

Replace quoted forward-ref union strings (e.g. "X | None") with unquoted
PEP 604 annotations by adding `from __future__ import annotations` to the
eight affected modules. Annotation-only imports are moved into TYPE_CHECKING
blocks to satisfy the repo's flake8-type-checking (TCH) rules; runtime-needed
imports stay at module level. No behavior changes.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jsong468 jsong468 self-assigned this Jul 1, 2026

@jsong468 jsong468 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.

Is there a way to ensure linters catch these inconsistencies in the future?

@romanlutz

Copy link
Copy Markdown
Contributor Author

Is there a way to ensure linters catch these inconsistencies in the future?

Yes. Pre-commit needs to run on all files, not just changed ones. I have another PR for that.

@romanlutz romanlutz added this pull request to the merge queue Jul 2, 2026
Merged via the queue into microsoft:main with commit 34f0073 Jul 2, 2026
53 checks passed
@romanlutz romanlutz deleted the romanlutz-modernize-typing-syntax branch July 2, 2026 04:35
romanlutz pushed a commit to romanlutz/PyRIT that referenced this pull request Jul 2, 2026
Adopt main's ScenarioContext scenario-building architecture (microsoft#2107) and
PEP 604 typing modernization (microsoft#2109), re-applying this PR's pre-1.0.0
deprecation removals on top. Resolved conflicts in memory storage
serializers/storage (removed deprecated alias methods), models __all__,
atomic_attack imports, and the airt/foundry/garak scenarios plus the
red_team_agent test.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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.

3 participants