docs: correct three docstring defaults that disagree with the signatures - #2618
Open
avalyset wants to merge 1 commit into
Open
docs: correct three docstring defaults that disagree with the signatures#2618avalyset wants to merge 1 commit into
avalyset wants to merge 1 commit into
Conversation
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.
Motivation
Three docstrings in the repository state parameter defaults that the function signatures do not have. A reader trusting the docstring would expect different behaviour than the code produces. The goal is to make those three lines agree with the signatures they describe.
Modification
opencompass/openicl/icl_evaluator/code_evaluator.pyretryretry: int = 5opencompass/models/turbomind_api.pytemperaturetemperature: float = 1.0opencompass/datasets/SciReasoner/Mol_Instructions/normalized_SW_score.pymatrix_namematrix_name='BLOSUM45'The
matrix_nameone is the most consequential of the three: BLOSUM62 and BLOSUM45 are different substitution matrices, so a reader trusting the docstring would expect different alignment scores than the function actually produces.Docstring text only — three lines, no behaviour changes, and no other issues in these files were touched.
Checked and left alone
While looking for this class of drift I also examined four other candidates and found the docstrings correct:
opencompass/utils/network.py—hostreads(default: Google DNS "8.8.8.8"), which matcheshost: str = '8.8.8.8'.opencompass/summarizers/llm_compression.pyandopencompass/summarizers/subjective/compass_arena_bradley_terry.py—time_strreads "Defaults todatetime.now().strftime('%Y%m%d_%H%M%S')", wrapped across two lines, matching the signature exactly.opencompass/openicl/icl_evaluator/lm_evaluator.py—seedstates no default at all, so there is nothing to disagree with. Its signature default is666, which is undocumented, but that is a gap rather than a contradiction and is left for a separate change.One related thing I noticed but did not touch, since it is not a default:
turbomind_api.pydocumentsend_strin theArgs:block ofgenerate, butend_stris not a parameter of that method — it is read fromself.end_str. Happy to fix that here or separately if you would like.BC-breaking (Optional)
No. The change is docstring text only; no signature, default, or runtime behaviour is altered, so nothing downstream can observe it.
Use cases (Optional)
Not a new feature.
Checklist
Before PR:
After PR:
Notes on the boxes left unchecked, so you are not left guessing:
On linting: all three files compile and pass
ruff check --select E9,F. The three pre-existing extraneous-f-prefix warnings thatruff --isolatedreports in these files are present onmainas well and are unrelated to this change.On the CLA checklist item: I could not find a CLA document, bot, or status check in this repository, and recent PRs do not appear to have gone through one. All commits here are authored and committed as
eirik@ecodeco.noand are GPG-verified. If there is a CLA process I have missed, point me to it and I will sign it.