Skip to content

fix: keep PR07-PR09 checks for option-set parameter types - #705

Open
dylanpulver wants to merge 1 commit into
numpy:mainfrom
dylanpulver:fix-pr07-09-option-set-params
Open

fix: keep PR07-PR09 checks for option-set parameter types#705
dylanpulver wants to merge 1 commit into
numpy:mainfrom
dylanpulver:fix-pr07-09-option-set-params

Conversation

@dylanpulver

Copy link
Copy Markdown

The options-set guard from #347 suppresses PR06 with a continue, which advances the parameter loop and skips the _check_desc call at the end of the loop body. Parameters typed as an options set have gone unchecked for PR07 through PR09 since b5b7e25.

Minimal pair, differing only in the type line:

bar : str          ->  PR07
bar : {'a', 'b'}   ->  no error

Constraint on the fix: the order of errs is asserted by several test_bad_docstrings expectations. Moving the _check_desc call above the type checks removes the same symptom but reorders codes within errs, so I negated the guard instead, which leaves the PR06 suppression and the emission order untouched.

Residual limitation: the guard keys on { appearing anywhere in the type, so a malformed type containing a brace now gets description checks it escaped before. That matches how every non-options type is treated, and narrowing it would change PR06 behaviour beyond the scope of this fix.

Measurement: 315 passed before, 317 after, on Python 3.12. Both added cases fail with validate.py reverted. valid_options_in_parameter_description_sets continues to pin the PR06 suppression from #347.

Worth weighing before a release: five suppressed findings reappear in matplotlib, among them pyplot.hlines linestyles (no description) and pyplot.quiver units (PR09).

Prepared with AI assistance.

The guard added in numpy#347 suppresses the common-type (PR06) checks when a
parameter type is a set of options. It does so with a continue, which
advances the parameter loop and therefore also skips the _check_desc call
at the end of the loop body, silently disabling PR07, PR08 and PR09 for
those parameters.

Negate the guard instead so only the PR06 block is skipped.
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.

1 participant