DOC: Fix parameter names in private helper docstrings - #14134
Open
karpovantonme wants to merge 3 commits into
Open
DOC: Fix parameter names in private helper docstrings#14134karpovantonme wants to merge 3 commits into
karpovantonme wants to merge 3 commits into
Conversation
karpovantonme
requested review from
agramfort,
drammock and
larsoner
as code owners
August 4, 2026 19:31
larsoner
reviewed
Aug 4, 2026
larsoner
left a comment
Member
There was a problem hiding this comment.
FYI we have started gradually removing params from many of these docstrings because they tend to get out of date quickly. That said, these seem like pretty straightforward improvements so I'm okay with merging once the changelog issue is addressed
Ten docstrings name a parameter that is not in the signature, so help() and editor tooltips show an argument that raises TypeError. Renames, one to one: - _lin_field_coeff and _do_lin_field_coeff: rmag/cosmag -> rmags/cosmags. The line just below already says 'for each rmag/cosmag/w', so the singular there is the element, the parameter is the plural. - write_named_matrix: matkind -> mat. mat was undocumented. - text2d: x/y -> x_window/y_window. - _cart_to_sph: cart_pts -> cart. - _sss_basis: coils -> all_coils. And one wrong default: _ajd_pham documents max_iter=1000, the signature has 15. pyRiemann, which the comment credits as the source, uses 20. Docstrings only.
karpovantonme
force-pushed
the
doc/param-names-match-signature
branch
from
August 5, 2026 12:24
52befad to
a84cfcd
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ten docstrings name a parameter the function does not take. All in private helpers, so nothing here is user facing, but
help()and editor tooltips are what a developer reads when working on this code.Renames, one to one
_lin_field_coeff,_do_lin_field_coeffrmag,cosmagrmags,cosmagswrite_named_matrixmatkindmat_abstract.text2dx,yx_window,y_window_cart_to_sphcart_ptscart_sss_basiscoilsall_coilsThe first pair is nice: the line right under it already reads "The sensor assignments for each rmag/cosmag/w", so the singular is the element and the parameter is the plural.
write_named_matrixdocumentedmatkind, which does not exist, and left the realmatundocumented.One wrong default
_ajd_phamdocumentsmax_iter : int, default 1000, the signature ismax_iter=15. That is off by a factor of 66, and it is the kind of number someone reads before deciding whether the algorithm converged. pyRiemann, which the comment right below credits as the source of this implementation, usesn_iter_max=20, so 1000 does not come from there either. I did not touch the code, only the number in the docstring.Ten more I did not touch
The same sweep flags ten where the parameter was not renamed but removed or split, so the fix is a judgement call rather than a rename:
_fft_multiply_repeateddocumentsh_fftandn_fft, both now live insidecuda_dict_prep_field_computationdocumentsrrandfwd_data, signature is(sensors, bem, n_jobs, verbose)_sph_to_cart_partialsdocumentssph_grads, which became five separate argumentscalculate_lutdocumentslim_cmap, gone entirely_fast_sphere_dot_r0(rr1),_bcd(n_positions),_read_segment(projector),_n_colors(bytesvsbytes_)Those need someone who knows whether the description should be dropped or rewritten for the new arguments. Point at any of them and I will send a follow-up.
Checks
Docstrings only, no code touched. The sweep reported 19 before and 10 after, and the ten left are exactly the list above.
AI assistance, per the policy: the mismatches were surfaced by a sweep that compares numpydoc
Parametersnames against the actual signature, run with Claude Opus in Claude Code, which also applied the renames and looked up the pyRiemann default. Each one was checked against the parsed signature before the edit, and I read the diff before submitting. Same setup as #14124 and #14125.