Repository navigation
Conversation
leo-collins
reviewed
Oct 30, 2026
leo-collins
left a comment
leo-collins
left a comment
Contributor
There was a problem hiding this comment.
This is fine pending my comment. Thanks for catching this!
Comment on lines
+377
to
+378
| Only the default value 1 is supported. Other values raise | ||
| ``NotImplementedError``. |
Contributor
There was a problem hiding this comment.
Can this also explain what adj_input actually does?
Contributor
|
ACtually I think this should be merged into the release branch, can you rebase and target that branch? |
The Parameters underline in L2TransformedFunctional.__call__ and L2TransformedFunctional.derivative was one dash short, so numpydoc did not read the section. The entries also named the arguments value and adj_value, while the methods take values and adj_input.
Assisted-by: Claude:claude-opus-5-5
mmporong
force-pushed
the
docs-l2transformedfunctional-params
branch
from
October 30, 2026 13:35
1df8a93 to
be5ca57
Compare
Author
|
I rebased onto release and updated the target branch. I also added an explanation of adj_input. |
This branch has not been deployed
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.
Description
The docstrings of
L2TransformedFunctional.__call__andL2TransformedFunctional.derivativeinfiredrake/adjoint/transformed_functional.pyhad two problems in their Parameters sections:Parametershad nine dashes, one fewer than the heading. numpydoc does not read a section with a short underline: it warns "potentially wrong underline length" and returns no parameters for either method.valueandadj_value, but the methods takevaluesandadj_input.This fixes both underlines and both names. The
adj_inputentry now also says what the code checks: only the default value 1 is supported, and other values raiseNotImplementedError.I found the
adj_valueentry by comparing the documented parameter names with the method signatures, and the other two problems while reading the same docstrings. Checks:NumpyDocStringon both docstrings: before, no parameters and the underline warning; after,['values']and['adj_input', 'apply_riesz'], the same as the signatures.python -m flake8 firedrake/adjoint/transformed_functional.pypasses with the repository'ssetup.cfg.Only docstrings change. I did not run the test suite, since no code changes.
AI assistance: this PR was written with Claude Code (Claude Opus 5.5). I can't add labels to this repository, so could a maintainer please apply the
LLM usedlabel?