MAINT: Enable ruff UP007/UP045 and modernise Optional/Union to PEP 604#1884
Open
romanlutz wants to merge 8 commits into
Open
MAINT: Enable ruff UP007/UP045 and modernise Optional/Union to PEP 604#1884romanlutz wants to merge 8 commits into
romanlutz wants to merge 8 commits into
Conversation
Aligns the codebase with the project style guide's mandated PEP 604 syntax (`X | None` instead of `Optional[X]`, `X | Y` instead of `Union[X, Y]`). Reverses the deliberate ignore from PR microsoft#1389 that predated the style-guide rule. - Removes UP007 and UP045 from the ruff ignore list - Auto-fix applied via `ruff check --fix` (no unsafe fixes needed) - Manual fix for two recursive Union[...] type aliases in pyrit/setup/configuration_loader.py that ruff cannot rewrite (RHS expressions rather than annotations) - Re-formatted with `ruff format` - ty diagnostic count unchanged (1376); full pytest tests/unit passes (8566/8566) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Resolved conflicts by taking main's version for files that were
heavily rewritten or refactored on main, then re-applying the
ruff PEP 604 sweep to pick up any Optional/Union usage in the
newly-merged code.
Conflicted files (all resolved by `git checkout --theirs` then
`ruff check --fix`):
- pyrit/analytics/result_analysis.py (main added IdentifierFilter support)
- pyrit/backend/mappers/attack_mappers.py (main changed default from None to {})
- pyrit/identifiers/component_identifier.py (main reduced to re-export shim)
- pyrit/identifiers/evaluation_identifier.py (main reduced to re-export shim)
- pyrit/models/message_piece.py (main refactored to Pydantic)
- tests/unit/analytics/test_result_analysis.py (main added IdentifierFilter tests)
The new `pyrit/models/identifiers/` package added by main also
got the PEP 604 sweep applied via `ruff --fix` (~20 violations).
One runtime use of `Optional[dict]` in
`pyrit/models/message_piece.py` (Pydantic `PlainSerializer
return_type=...`) was hand-fixed to `dict | None` since ruff
won't auto-rewrite RHS runtime values.
Verification:
- ruff check pyrit/ tests/ doc/ - clean
- ruff format --check - clean
- pytest tests/unit -n 4 - 8913 passed, 5 skipped, 0 failures
- ty check - 1426 diagnostics vs 1412 on main (+14 net). Spot-
checked deltas: `ty` becomes slightly more aggressive at
resolving forward refs after Optional imports are dropped,
exposing latent issues (e.g. test passing `page=None` to test
RuntimeError, AzureML SDK types). None are real regressions.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The prior merge commit (3f1e734) was structurally a merge commit but only included new files from main, not the modifications to existing files - they were stashed during conflict resolution and never re-staged before commit. This commit adds back the missing content (317 files) so the branch state actually matches what `git merge origin/main` would have produced. No code or test changes here vs the intended merge state - verified via `ruff check` clean and the file content matches `origin/main` for non-typing-modified files. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Brings in 3 new commits from main: - 648faa9 FEAT: Backfill class-level metadata for all remote seed datasets (microsoft#1780) - 092126d MAINT: Migrate AddImage/AddTextImage converter deprecations to print_deprecation_message (microsoft#1875) - 376e000 FEAT: Add TatweelConverter for Arabic kashida insertion (microsoft#1869) Conflict resolution (11 files): took main's version everywhere (`git checkout --theirs`), then re-ran `ruff check --fix` to re-apply the PEP 604 sweep to main's new code (~36 violations auto-fixed). Same hand-fix for the runtime `Optional[dict]` in `pyrit/models/message_piece.py` PlainSerializer `return_type` that ruff can't auto-rewrite. Verification: - ruff check pyrit/ tests/ doc/ - clean - ruff format --check - clean - pytest tests/unit -n 4 - 8977 passed, 5 skipped, 0 failures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…kticks The typing modernization sweep in this PR rewrites `Optional[str]` to `str | None` in `pyrit/models/conversation_reference.py` and `pyrit/models/retry_event.py`. That puts both files in the changed-files set CI feeds to `pre-commit run --from-ref origin/main --to-ref HEAD`, which surfaces four pre-existing `:meth:` reST roles (landed via microsoft#1769 before the `check-no-rest-roles` hook from microsoft#1823 existed). Replace `:meth:model_dump` and `:meth:model_validate` with plain double-backticks so the hook passes. `model_dump` / `model_validate` are Pydantic methods, so the auto-linker in `build_scripts/gen_api_md.py` correctly leaves them as plain code spans. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Brings in 18 new commits from main, including several large refactors: - 13a1da6 [BREAKING] MAINT: enforce _async suffix on async functions - f43855b MAINT: Making Message a Pydantic model - b3b018f MAINT: Making Score a Pydantic model - 414ac8c MAINT: Migrate os.path.* to pathlib.Path in pyrit/ - d6173f1 MAINT: Eliminate blocking I/O on async code paths - 9bb005f TEST: stop GCG unit tests from hitting HuggingFace - 17 conflicts, all resolved by `git checkout --theirs` then re-running `ruff check --fix` to apply the PEP 604 sweep to main's new code (~149 violations auto-fixed in newly-merged files). No manual hand-fixes needed this time - the runtime `Optional[dict]` in `pyrit/models/message_piece.py` that needed hand-fixing in prior merges is gone now that Message and MessagePiece are full Pydantic models on main. Verification: - ruff check pyrit/ tests/ doc/ - clean - ruff format --check - clean - pytest tests/unit -n 4 - 9273 passed, 6 skipped, 0 failures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Brings in 1 new commit from main: - 729edfa BUG: Stop leaking absolute media paths and SAS tokens in Attack History 'Last Message' (microsoft#1865) One conflict in `pyrit/models/conversation_stats.py` resolved by `git checkout --theirs` then `ruff check --fix` to apply the PEP 604 sweep (4 violations auto-fixed in the merged code). Verification: - ruff check pyrit/ tests/ doc/ - clean - ruff format --check - clean - pytest tests/unit -n 4 - 9305 passed, 6 skipped, 0 failures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Completes the PEP 604 modernization for usages that survived earlier sweeps (mostly arrived via post-modernization merges from main). Also guards three pre-existing None-deref hot spots in tree_of_attacks and crescendo unmasked by the recent Score Pydantic refactor, which now have to be addressed because this branch touches those files and the incremental ty-check pre-commit hook reports them. Fixes: - Optional[X] -> X | None in function signatures and local annotations across prompt_normalizer, openai_realtime_target, anecdoctor, and add_image_to_video_converter - crescendo.py: guard refusal_score.score_rationale[:100] with or ''` - tree_of_attacks.py: guard score.scorer_class_identifier.class_name with fallback `'unknown'` - tree_of_attacks.py: guard on_topic_score.score_rationale with or ''` Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
The PyRIT style guide mandates PEP 604 / PEP 585 typing (
X | None,X | Y,list[X],dict[K, V]), butpyproject.tomlwas still ignoring the ruff rules that enforce the union/optional half of that (UP007,UP045). PR #1389 added the ignore back in Feb 2026 per a "team preference to preserveOptional/Union" that has since been superseded by the style guide. This sweep removes the ignore and aligns the codebase with the style guide.Approach
Mechanical, ruff-driven, zero logic change:
UP007andUP045from[tool.ruff.lint] ignoreso the rules are enforced going forward.ruff check --fixauto-rewrote 1803 of 1809 violations (Optional[X]->X | None,Union[X, Y]->X | Y, plusF401cleanup of now-unusedOptional/Unionimports in the same pass). No--unsafe-fixesneeded.pyrit/setup/configuration_loader.py(YamlPrimitive,YamlValue) - ruff doesn't auto-rewrite these because they are RHS runtime values rather than annotations.ruff formatreflowed 21 files where the shorter annotation lets the signature collapse onto fewer lines.Min Python is 3.10 and pydantic is 2.13.4, so PEP 604 is fully supported at runtime everywhere - no
from __future__ import annotationsneeded.Things to know
ruff formatreflow boundary (e.g.filter_harm_categories: Optional[list[Literal[...]]] = Noneon N lines becomingfilter_harm_categories: list[Literal[...]] | None = Noneon N-1 lines). No identifier renames, no behavioural edits.cast(""Optional[str]"", ...),""Optional[Sequence[...]]"") are handled correctly by ruff - the inner forward ref is preserved, only the outerOptional[...]is lifted.pyrit/backend/services/converter_service.pyusesUnionandOptionalat runtime (if origin is Union:,f""Optional[{inner}]""). Ruff correctly leaves those alone; theUnionimport stays.Tests and Documentation
ruff check pyrit/ tests/ doc/- cleanruff format --check- cleanty check- 1376 diagnostics, exact match to the pre-sweep baseline (no new type errors)pytest tests/unit -n 4- 8566 passed, 5 skipped, 0 failuresdoc/getting_started/troubleshooting/{deploy,download_and_register}_hf_model_aml.{py,ipynb}) updated in-place by ruff; cell outputs preserved.No new tests or doc updates - the change is mechanical and not user-facing.