Skip to content

[pydocstyle] Refactor D417 to use the pydoclint docstring processing#21154

Open
augustelalande wants to merge 7 commits intoastral-sh:mainfrom
augustelalande:d417-refactor
Open

[pydocstyle] Refactor D417 to use the pydoclint docstring processing#21154
augustelalande wants to merge 7 commits intoastral-sh:mainfrom
augustelalande:d417-refactor

Conversation

@augustelalande
Copy link
Contributor

@augustelalande augustelalande commented Oct 30, 2025 •
edited
Loading

Summary

This PR refactors D417 to use the parameters detected in the pydoclint scope, the goal is to have a D417 implementation which is equivalent to the previous one, but uses the pydoclint docstring processing. Later on D417 can be renamed to DOC101 and D417 deprecated. This will make deploying DOC101 easier.

Test Plan

The refactor is tested on the old fixtures, and the new snap was manually diffed. The following changes were found between the old and new implementation.

In the following the new implementation reports an error which the old didn't. Notice that the code mixes google and numpy style elements which makes the docstring invalid for both.

def f(x):
    """Do something with valid description.

    Args:
    ----
        x: the value

    Returns:
    -------
        the value
    """
    return x

In the following, the old implementation reports an error but the new one does not. This is because the old implementation enforces argument order in the docstring which the new one does not. Pydoclint has a seperate rule for checking documentation order which can be implemented later.

    @classmethod
    @expect("D417: Missing argument descriptions in the docstring "
            "(argument(s) test, y, z are missing descriptions in "
            "'test_missing_args_class_method' docstring)", arg_count=4)
    def test_missing_args_class_method(cls, test, x, y, z=3):  # noqa: D213, D407
        """Test a valid args section.

        Parameters
        ----------
        z
        x
            Another parameter. The parameters y, test below are
            missing descriptions. The parameter z above is also missing
            a description.
        y
        test

        """

@augustelalande augustelalande marked this pull request as draft October 30, 2025 23:52
@augustelalande augustelalande changed the title [pydocstyle] Refactor D417 to use the pydoclint scope [pydocstyle] Refactor D417 to use the pydoclint docstring processing Oct 30, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Oct 31, 2025 •
edited by astral-sh-bot bot
Loading

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+3 -7 violations, +0 -0 fixes in 2 projects; 53 projects unchanged)

apache/superset (+0 -5 violations, +0 -0 fixes) 

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview --select ALL

- scripts/erd/erd.py:84:5: D417 Missing argument descriptions in the docstring for `introspect_sqla_model`: `mapper`, `seen`
- superset/migrations/shared/catalogs.py:131:5: D417 Missing argument descriptions in the docstring for `print_processed_batch`: `batch_size`, `model`, `offset`, `start_time`, `total_rows`
- superset/migrations/shared/catalogs.py:166:5: D417 Missing argument descriptions in the docstring for `update_catalog_column`: `catalog`, `database`, `downgrade`, `session`
- superset/migrations/shared/catalogs.py:295:5: D417 Missing argument descriptions in the docstring for `delete_models_non_default_catalog`: `catalog`, `database`, `session`
- superset/utils/pandas_postprocessing/histogram.py:24:5: D417 Missing argument descriptions in the docstring for `histogram`: `bins`, `column`, `cumulative`, `df`, `groupby`, `normalize`

bokeh/bokeh (+3 -2 violations, +0 -0 fixes) 

ruff check --no-cache --exit-zero --no-fix --output-format concise --preview --select ALL

- src/bokeh/core/has_props.py:421:9: D417 Missing argument description in the docstring for `set_from_json`: `value`
+ src/bokeh/core/has_props.py:421:9: D417 Missing argument descriptions in the docstring for `set_from_json`: `setter`, `value`
+ src/bokeh/document/models.py:257:9: D417 Missing argument descriptions in the docstring for `update_name`: `new_name`, `old_name`
- src/bokeh/model/model.py:326:9: D417 Missing argument descriptions in the docstring for `js_link`: `attr_selector`, `other`, `other_attr`
+ src/bokeh/resources.py:194:5: DOC501 Raised exception `RuntimeError` missing from docstring

Changes by rule (2 rules affected)

code total + violation - violation + fix - fix
D417 9 2 7 0 0
DOC501 1 1 0 0 0

@augustelalande augustelalande marked this pull request as ready for review November 1, 2025 06:20
@augustelalande
Copy link
Contributor Author

augustelalande commented Nov 12, 2025

The ecosystem changes are minimal, with the new implementation removing some old false positives. It introduces some new positives, caused by improper formatting which are debatably true positives in my opinion.

@ntBre ntBre self-requested a review November 13, 2025 22:11
@ntBre
Copy link
Contributor

ntBre commented Nov 13, 2025

Thanks for the merge and taking a look at the ecosystem results. This is in my inbox to have a look!

@ntBre ntBre mentioned this pull request Feb 25, 2026
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ntBre ntBre Awaiting requested review from ntBre

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@augustelalande @ntBre