Skip to content

Reconcile the drifted parameter docstrings - #212

Merged
sappelhoff merged 3 commits into
mainfrom
reconcile-parameter-docs
Aug 21, 2026
Merged

Reconcile the drifted parameter docstrings#212
sappelhoff merged 3 commits into
mainfrom
reconcile-parameter-docs

Conversation

@sappelhoff

@sappelhoff sappelhoff commented Aug 21, 2026

Copy link
Copy Markdown
Owner

Documentation only — no change to behaviour, signatures or defaults.

Several parameters are documented in more than one place, because their
descriptions were copy-pasted between NoisyChannels, PrepPipeline,
Reference, find_bad_by_ransac and removeTrend. The copies have since
drifted: markup was lost, a cross-reference was dropped, and one type string
is wrong in twelve places. This reconciles them.

Type strings

ransac, channel_wise and matlab_strict were rendered as bool | None in
twelve places. All twelve have a plain bool default and never accept None, so
the API docs told the reader they could pass it. They now say bool.

The entries that keep bool | None are the ones in
NoisyChannels.find_all_bads, where None really does mean "use the value
from instantiation", or where the parameter is documented as ignored.

Prose

  • ransac in PrepPipeline and Reference had lost the sentence warning that
    RANSAC slows noisy channel detection down considerably, and left its default
    unmarked.
  • random_state in PrepPipeline, Reference and find_bad_by_ransac said
    "see RandomState for details" without linking anything, and marked up none of
    its literals. All three now link ~numpy.random.RandomState.
  • matlab_strict in Reference was the only description of that flag in the
    package that did not point at :ref:matlab-diffs``, the page listing the
    improvements it turns off.
  • reject_by_annotation in Reference no longer said what 'omit' does to the
    data or why one would ask for it.
  • interpolate_bads in PrepPipeline.robust_reference now names the
    alternative to interpolating, as the Reference copy already did.

Each site keeps the opening sentence that is genuinely its own: what the random
seed is used for differs between the pipeline, the reference and RANSAC, and
matlab_strict is described in terms of the code the reader is looking at
("PyPREP", "RANSAC", "detrending").

The wording fixes get no changelog entry, but the type strings do: a user
reading the rendered API was told three boolean flags accept None.

Open questions

  • Based on quiet-logging-by-default, not main. Do not configure logging when pyprep is imported #211 is still open and
    touches two of the same files, though not these lines. Retarget to main
    once it lands.
  • No shared docdict. Repeating a parameter description per function is
    right for numpydoc — a reader of NoisyChannels.__init__ wants the text
    inline, not a pointer elsewhere. Deduplicating for real means an MNE-style
    docdict + fill_doc substituting one canonical string at import time,
    which is a separate and much larger change. Until then these copies can drift
    again.
  • {int, None} | None and {None, 'omit'} | None left alone. The trailing
    | None is redundant in both, and in every similar type string in the
    package. Untangling that is its own decision, not part of a wording fix.
  • PrepPipeline.robust_reference documents max_iterations by deferring to the
    prep_params dict, while Reference states the number outright. Both
    readings are defensible for their own call signature, so neither was touched.
  • Descriptions that differ deliberately were left differing: channel_wise and
    max_chunk_size drop the "has no effect if not using RANSAC" clause inside
    the RANSAC functions themselves, and the private RANSAC helpers do not render
    in the docs at all.

Verification

pytest (61 passed), pre-commit run --all-files, and a clean
make html in docs/ with no warnings.

@sappelhoff
sappelhoff deleted the branch main August 21, 2026 09:37
@sappelhoff sappelhoff closed this Aug 21, 2026
@sappelhoff sappelhoff reopened this Aug 21, 2026
@sappelhoff
sappelhoff changed the base branch from quiet-logging-by-default to main August 21, 2026 09:37
The rendered API listed `ransac`, `channel_wise` and `matlab_strict` as
`bool | None` in twelve places. All twelve have a plain bool default and
never accept `None`, so a reader was told they could pass it. The four
entries that keep `bool | None` are the ones in `find_all_bads`, where
`None` really does mean "use the value from instantiation".
`ransac`, `random_state`, `matlab_strict` and `reject_by_annotation` are
each documented in several places, and the copies had drifted apart. The
`PrepPipeline` and `Reference` copies of `ransac` had lost the sentence
warning that RANSAC slows detection down considerably; three copies of
`random_state` pointed at "RandomState" without linking it and left every
literal unmarked; `Reference` neither marked up the `matlab_strict`
default nor linked the page listing the differences it turns off; and its
`reject_by_annotation` no longer said what `'omit'` does to the data or
why one would ask for it.

Each site keeps the opening sentence that is genuinely its own: what the
seed is used for differs between the pipeline, the reference and RANSAC,
and `matlab_strict` is described in terms of the code the reader is
looking at.
The wording fixes need no entry, but a user reading the rendered API was
told that three boolean flags accept `None`.
@sappelhoff
sappelhoff force-pushed the reconcile-parameter-docs branch from af89c6d to 9577130 Compare August 21, 2026 09:37
@codecov

codecov Bot commented Aug 21, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 98.03%. Comparing base (64f1fef) to head (9577130).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #212   +/-   ##
=======================================
  Coverage   98.03%   98.03%           
=======================================
  Files           8        8           
  Lines         864      864           
=======================================
  Hits          847      847           
  Misses         17       17           

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sappelhoff
sappelhoff merged commit d4d2eb5 into main Aug 21, 2026
10 checks passed
@sappelhoff
sappelhoff deleted the reconcile-parameter-docs branch August 21, 2026 09:44
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