Skip to content

Comments

[pydocstyle] Suppress D417 for parameters with Unpack annotations#21816

Merged
ntBre merged 6 commits intoastral-sh:mainfrom
phongddo:phongddo/d417-unpack-args
Dec 8, 2025
Merged

[pydocstyle] Suppress D417 for parameters with Unpack annotations#21816
ntBre merged 6 commits intoastral-sh:mainfrom
phongddo:phongddo/d417-unpack-args

Conversation

@phongddo
Copy link
Contributor

@phongddo phongddo commented Dec 5, 2025 •
edited
Loading

Summary

Fixes #8774

This PR fixes pydocstyle incorrectly flagging missing argument for arguments with Unpack type annotation by extracting the kwarg D417 suppression logic into a helper function for future rules as needed.

Problem Statement

The below example was incorrectly triggering D417 error for missing **kwargs doc.

class User(TypedDict):
    id: int
    name: str

def do_something(some_arg: str, **kwargs: Unpack[User]):
    """Some doc
    
    Args:
        some_arg: Some argument
    """
image

**kwargs: Unpack[User] indicates the function expects keyword arguments that will be unpacked. Ideally, the individual fields of the User TypedDict should be documented, not in the **kwargs itself. The **kwargs parameter acts as a semantic grouping rather than a parameter requiring documentation.

Solution

As discussed in the linked issue, it makes sense to suppress the D417 for parameters with Unpack annotation. I extract a helper function to solely check D417 should be suppressed with **kwarg: Unpack[T] parameter, this function can also be unit tested independently and reduce complexity of current missing_args check function. This also makes it easier to add additional rules in the future.

✏️ Note: This is my first PR in this repo, as I've learned a ton from it, please call out anything that could be improved. Thanks for making this excellent tool 👏

Test Plan

Add 2 test cases in D417.py and update snapshots.

@phongddo phongddo marked this pull request as ready for review December 5, 2025 16:23
@phongddo phongddo mentioned this pull request Dec 5, 2025
Closed
@ntBre ntBre added rule Implementing or modifying a lint rule docstring Related to docstring linting or formatting labels Dec 5, 2025
@astral-sh-bot
Copy link

astral-sh-bot bot commented Dec 5, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

ntBre
ntBre requested changes Dec 5, 2025
View reviewed changes
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Thank you! This looks great to me overall, I just had a few very small nits. I think it may also be worth calling out this exception in the rule documentation, and possibly linking to the Python docs.

@phongddo
Copy link
Contributor Author

phongddo commented Dec 5, 2025

Hey @ntBre 👋 Thanks for reviewing the PR! I've addressed your comments and updated the rule documentation as you suggested. It's my first time writing a rule doc, so I'm not sure if it follows standard, would love your feedback 🙏

@phongddo phongddo requested a review from ntBre December 5, 2025 22:28
ntBre
ntBre approved these changes Dec 8, 2025
View reviewed changes
Copy link
Contributor

@ntBre ntBre left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

@ntBre ntBre changed the title [pydocstyle] Suppress D417 for parameters with Unpack annotation [pydocstyle] Suppress D417 for parameters with Unpack annotations Dec 8, 2025
@ntBre ntBre enabled auto-merge (squash) December 8, 2025 18:53
@ntBre ntBre merged commit 45fb373 into astral-sh:main Dec 8, 2025
36 checks passed
dcreager added a commit that referenced this pull request Dec 9, 2025
@dcreager
Merge branch 'dcreager/die-die-intersections' into dcreager/callable-…
1fe5ba4 
…return

* dcreager/die-die-intersections: (29 commits)
  simpler bounds
  [`pylint`] Detect subclasses of builtin exceptions (`PLW0133`) (#21382)
  Fix stack overflow with recursive generic protocols (depth limit) (#21858)
  New diagnostics for unused range suppressions (#21783)
  [ty] Use default settings in completion tests
  [ty] Infer type variables within generic unions  (#21862)
  [ty] Fix overload filtering to prefer more "precise" match (#21859)
  [ty] Stabilize auto-import
  [ty] Fix reveal-type E2E test (#21865)
  [ty] Use concise message for LSP clients not supporting related diagnostic information (#21850)
  Include more details in Tokens 'offset is inside token' panic message (#21860)
  apply range suppressions to filter diagnostics (#21623)
  [ty] followup: add-import action for `reveal_type` too (#21668)
  [ty] Enrich function argument auto-complete suggestions with annotated types
  [ty] Add autocomplete suggestions for function arguments
  [`flake8-bugbear`] Accept immutable slice default arguments (`B008`) (#21823)
  [`pydocstyle`] Suppress `D417` for parameters with `Unpack` annotations (#21816)
  [ty] Remove legacy `concise_message` fallback behavior (#21847)
  [ty] Make Python-version subdiagnostics less verbose (#21849)
  [ty] Supress inlay hints when assigning a trivial initializer call (#21848)
  ...
dcreager added a commit that referenced this pull request Dec 9, 2025
@dcreager
Merge branch 'dcreager/die-die-intersections' into dcreager/callable-…
5a5474f 
…return

* dcreager/die-die-intersections: (31 commits)
  clippy
  reword comment
  simpler bounds
  [`pylint`] Detect subclasses of builtin exceptions (`PLW0133`) (#21382)
  Fix stack overflow with recursive generic protocols (depth limit) (#21858)
  New diagnostics for unused range suppressions (#21783)
  [ty] Use default settings in completion tests
  [ty] Infer type variables within generic unions  (#21862)
  [ty] Fix overload filtering to prefer more "precise" match (#21859)
  [ty] Stabilize auto-import
  [ty] Fix reveal-type E2E test (#21865)
  [ty] Use concise message for LSP clients not supporting related diagnostic information (#21850)
  Include more details in Tokens 'offset is inside token' panic message (#21860)
  apply range suppressions to filter diagnostics (#21623)
  [ty] followup: add-import action for `reveal_type` too (#21668)
  [ty] Enrich function argument auto-complete suggestions with annotated types
  [ty] Add autocomplete suggestions for function arguments
  [`flake8-bugbear`] Accept immutable slice default arguments (`B008`) (#21823)
  [`pydocstyle`] Suppress `D417` for parameters with `Unpack` annotations (#21816)
  [ty] Remove legacy `concise_message` fallback behavior (#21847)
  ...
dcreager added a commit that referenced this pull request Dec 9, 2025
@dcreager
Merge branch 'dcreager/die-die-intersections' into dcreager/callable-…
e99fe24 
…return

* dcreager/die-die-intersections: (31 commits)
  clippy
  reword comment
  simpler bounds
  [`pylint`] Detect subclasses of builtin exceptions (`PLW0133`) (#21382)
  Fix stack overflow with recursive generic protocols (depth limit) (#21858)
  New diagnostics for unused range suppressions (#21783)
  [ty] Use default settings in completion tests
  [ty] Infer type variables within generic unions  (#21862)
  [ty] Fix overload filtering to prefer more "precise" match (#21859)
  [ty] Stabilize auto-import
  [ty] Fix reveal-type E2E test (#21865)
  [ty] Use concise message for LSP clients not supporting related diagnostic information (#21850)
  Include more details in Tokens 'offset is inside token' panic message (#21860)
  apply range suppressions to filter diagnostics (#21623)
  [ty] followup: add-import action for `reveal_type` too (#21668)
  [ty] Enrich function argument auto-complete suggestions with annotated types
  [ty] Add autocomplete suggestions for function arguments
  [`flake8-bugbear`] Accept immutable slice default arguments (`B008`) (#21823)
  [`pydocstyle`] Suppress `D417` for parameters with `Unpack` annotations (#21816)
  [ty] Remove legacy `concise_message` fallback behavior (#21847)
  ...
@BrewTestBot BrewTestBot mentioned this pull request Dec 11, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ntBre ntBre ntBre approved these changes

Assignees

No one assigned

Labels

docstring Related to docstring linting or formatting rule Implementing or modifying a lint rule

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

[D417] Documenting Functions that Unpack

2 participants

@phongddo @ntBre