Skip to content

docs/types(pt2e): annotate and document DuplicateDQPass (#4541) - #4565

Merged
andrewor14 merged 1 commit into
pytorch:mainfrom
Anai-Guo:type-hints/pt2e-duplicate-dq-pass
Jul 22, 2026
Merged

docs/types(pt2e): annotate and document DuplicateDQPass (#4541)#4565
andrewor14 merged 1 commit into
pytorch:mainfrom
Anai-Guo:type-hints/pt2e-duplicate-dq-pass

Conversation

@Anai-Guo

Copy link
Copy Markdown
Contributor

Summary

Part of the PT2E type-hinting & docstring audit proposed in #4541. This PR
covers a single small, self-contained file:
torchao/quantization/pt2e/quantizer/duplicate_dq_pass.py.

  • Remove the file-level # mypy: allow-untyped-defs.
  • Add the one missing annotation: _maybe_duplicate_dq(...) -> None (it was the
    only untyped def; DuplicateDQPass.call and the nested maybe_replace_node
    were already annotated).
  • Add module-, function-, class-, and method-level docstrings.

No behavior change — the executable lines are byte-for-byte identical; only the
mypy pragma, the one return annotation, and docstrings differ.

Why this is safe

allow-untyped-defs only suppresses checking of untyped function bodies, so
call was already type-checked and passing. The only newly-checked body is
_maybe_duplicate_dq, whose types are straightforward (GraphModule/Node
in, None out); the existing # type: ignore[assignment] on the
user.args/user.kwargs rewrites is preserved.

ruff check passes and no line exceeds 88 columns.

🤖 Generated with Claude Code

Type _maybe_duplicate_dq (the only untyped def) and drop the file-level
# mypy: allow-untyped-defs, and add module/function/class docstrings.
No behavior change. Part of the PT2E type-hint & docstring audit (pytorch#4541).
@pytorch-bot

pytorch-bot Bot commented Jul 11, 2026

Copy link
Copy Markdown

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/ao/4565

Note: Links to docs will display an error until the docs builds have been completed.

❗ 1 Active SEVs

There are 1 currently active SEVs. If your PR is affected, please view them below:

✅ No Failures

As of commit 53bc23b with merge base bfbc842 (image):
💚 Looks good so far! There are no failures yet. 💚

This comment was automatically generated by Dr. CI and updates every 15 minutes.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 11, 2026
@andrewor14

Copy link
Copy Markdown
Contributor

@claude review please

@claude

claude Bot commented Jul 21, 2026

Copy link
Copy Markdown

Claude finished @andrewor14's task in 37s —— View job


Code Review: DuplicateDQPass annotation & docs

  • Read the PR diff and full file
  • Verify no behavior change
  • Check annotation & docstring accuracy

Verdict: LGTM ✅ — This does exactly what it claims: adds one return annotation, drops the file-level # mypy: allow-untyped-defs, and adds docstrings. No executable line changed.

Correctness

  • No behavior change confirmed. The only functional lines touched are the signature of _maybe_duplicate_dq (annotation only) and the removed mypy pragma. The # type: ignore[assignment] on user.args/user.kwargs (lines 70–71) is preserved, which is correct — Node.args/kwargs are typed as immutable Argument tuples, so the assignments still need the ignore.
  • -> None is accurate. Both return paths (return at line 58 and falling off the end) yield None.
  • Docstrings match the code. The module/call docstrings correctly describe the choose_qparams -> getitem -> q -> dq dynamic-quant skip and the per-user cloning via annotation check. The _maybe_duplicate_dq docstring correctly notes unannotated users are left untouched.

Minor observations (non-blocking, not introduced by this PR)

  • q_node = node.args[0] (line 92) is accessed as q_node.op without a Node type guard, unlike the nested getitem_node/choose_qparam_node checks which do use isinstance(..., torch.fx.node.Node). Since allow-untyped-defs is now removed for the module, it's worth confirming mypy is happy with call here — but this line is inside the already-type-checked call body and unchanged by this PR, so if CI is green it's a non-issue. Not something to fix in this docs-focused PR.
  • Module docstring says "once per annotated user" while call's docstring says "cloned per user" — both are fine given the annotation gate lives inside _maybe_duplicate_dq; consistent enough.

Consistency with repo conventions

Nice, tidy change. No requested changes from me.
• branch type-hints/pt2e-duplicate-dq-pass

@andrewor14 andrewor14 added topic: documentation Use this tag if this PR adds or improves documentation module: pt2e_quant pt2 export quantization (prepare_pt2e, convert_pt2e, quantizer) and removed topic: documentation Use this tag if this PR adds or improves documentation labels Jul 21, 2026
@andrewor14
andrewor14 merged commit 1580a9e into pytorch:main Jul 22, 2026
18 of 20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. module: pt2e_quant pt2 export quantization (prepare_pt2e, convert_pt2e, quantizer)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants