Skip to content

Fix docstrings naming parameters that are not in the signature - #1003

Merged
MicahGale merged 1 commit into
idaholab:developfrom
karpovantonme:docs/docstring-parameter-names
Aug 4, 2026
Merged

Fix docstrings naming parameters that are not in the signature#1003
MicahGale merged 1 commit into
idaholab:developfrom
karpovantonme:docs/docstring-parameter-names

Conversation

@karpovantonme

@karpovantonme karpovantonme commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes #1002.

Four docstrings name a parameter the function does not take. Details and the reasoning for each are in the issue; in short:

  • symmetric_difference / symmetric_difference_update document others (plural, "the other collections") but take a single other — the text was copied from difference(*others) above, which really is varargs.
  • PaddingNode.append and ListNode.append document node; both signatures are append(self, val, ...).
  • Nuclide._parse_fancy_name has two Parameters entries for one argument: a bare identifier, then idenitifer with the letters transposed.
  • Surface.update_pointers documents data_cards; the signature is data_inputs, and Material.update_pointers and ThermalScatteringLaw.update_pointers already document it that way.

Docstrings only — no code, no behaviour change, so there is nothing for a test to assert. Say the word if you would rather have it covered some other way.


General Checklist

  • I have performed a self-review of my own code.
  • The code follows the standards outlined in the development documentation.
  • I have formatted my code with black version 25 or 26.
  • I have added tests that prove my fix is effective or that my feature works (if applicable) — not applicable, docstrings only.

LLM Disclosure

  1. Are you?

    • A human user
    • A large language model (LLM), including ones acting on behalf of a human
  2. Were any large language models (LLM or "AI") used in to generate any of this code?

  • Yes
    • Model(s) used: Claude Opus (Claude Code)
  • No

The four were found by comparing every NumPy-style Parameters block in the package against the actual signature, then reading each hit to rule out decorators, **kwargs and the property-setter documentation style used in fill.py and importance.py (those two are deliberate and were left alone).

Documentation Checklist

  • I have documented all added classes and methods.
  • I have added type hints to all functions as needed.
  • I have marked all changes with the .. versionchanged:: or .. versionadded:: directives — not applicable, no API change.

📚 Documentation preview 📚: https://montepy--1003.org.readthedocs.build/en/1003/


📚 Documentation preview 📚: https://montepy--1003.org.readthedocs.build/en/1003/

- symmetric_difference/_update take one `other`, not varargs `others`
- both `append` methods take `val`, not `node`
- _parse_fancy_name had a duplicated, transposed `idenitifer` entry
- Surface.update_pointers takes `data_inputs`, not `data_cards`

Fixes idaholab#1002

@MicahGale MicahGale left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

The changes look good for the most part. Though, @karpovantonme why did you remove your disclosure of using Claude code?

@karpovantonme

Copy link
Copy Markdown
Contributor Author

Sorry — that was carelessness with the form on my part, not an attempt to hide anything. I changed the first box to say I'm a human, since I am the one submitting this and standing behind it, and flipped the second one in the same edit without thinking it through. I've put it back.

To be explicit: yes, Claude Code was used. It ran the sweep that compares every numpydoc Parameters block in the package against the actual signature, and it wrote the patch. I read the diff before submitting and can explain each of the four changes — including why fill.py and importance.py are not in it, since documenting the setter's parameter in the property's docstring is deliberate there and Sphinx renders it from the getter.

@tjlaboss

tjlaboss commented Aug 3, 2026

Copy link
Copy Markdown
Collaborator

The changelog test failure can be ignored in this case.

@MicahGale MicahGale left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good. I agree we can ignore the changelog requirement. Thanks @karpovantonme!

@MicahGale
MicahGale merged commit 5091097 into idaholab:develop Aug 4, 2026
40 of 43 checks passed
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.

Four docstrings document parameter names that are not in the signature

3 participants