docs: type three boolean docstring parameters as bool, not str - #14724
Open
iridescentWen wants to merge 1 commit into
Open
docs: type three boolean docstring parameters as bool, not str#14724iridescentWen wants to merge 1 commit into
iridescentWen wants to merge 1 commit into
Conversation
Each of these lines pairs a `str` type with a boolean default, so it contradicts itself: - pipeline_utils.py:423 silence_dtype_warnings - modular_pipeline.py:2612 silence_dtype_warnings - convert_from_ckpt.py:1213 from_safetensors `str` misleads readers into passing "False", which is truthy. Fixes huggingface#14723 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
Fixes #14723
What's wrong
Three docstrings annotate a boolean parameter as
`str`. Each line contradicts itself, because itpairs the
strtype with a boolean default:pipelines/pipeline_utils.pysilence_dtype_warnings`str`boolmodular_pipelines/modular_pipeline.pysilence_dtype_warnings`str`boolpipelines/stable_diffusion/convert_from_ckpt.pyfrom_safetensors`str`boolstractively misleads: it implies passing"False", which is truthy.Evidence, from the code rather than the parameter names
silence_dtype_warnings— the same docstring's own signature examples, three lines above theentry, already write it as a boolean (
pipeline_utils.py:408,:410,:412:to(dtype, silence_dtype_warnings=False)). It is read askwargs.pop("silence_dtype_warnings", False)(:431), used as a bare condition (:588and not silence_dtype_warnings), and internal callers passTrue(:1230,:1344). The prosedirectly below it reads "Whether to omit warnings…".
from_safetensors— annotatedfrom_safetensors: bool = Falseat:1156, used as a barecondition at
:1269. The next docstring entry,load_safety_checker (`bool`, *optional*, defaults to `True`), is the correct spelling of the sameshape.
Validation
git diff --stat: 3 files, +3/-3.Self-review (per CONTRIBUTING)
Ran the
.ai/skills/self-reviewrubric against.ai/review-rules.md:# Copied from: checked per function, since two of the three fix the same parameter text intwo files — that is the shape a copy-link usually takes. Both are
def to(self, *args, **kwargs)(
pipeline_utils.py:397,modular_pipeline.py:2586) and neither carries a# Copied fromheader,so editing both by hand is correct rather than fixing one source and running
make fix-copies.utils/check_copies.pyis outside my sparse checkout, so this is the manual equivalent; CI'sconsistency check will confirm.
docs/page restates these types.How this was found, and what it deliberately excludes
An AST pass comparing each documented parameter type against its real annotation. It only reports
unambiguous contradictions between two simple types, and skips everything it cannot settle exactly:
unannotated parameters, containers/unions/generics on either side, prose types, and compatible
spellings like
boolvsint. After this change it reports 0 forsrc/diffusers.Related, deliberately left out — a different problem class that would touch 19 files: 27
docstrings write a boolean default as a quoted string, e.g.
force_zeros_for_empty_prompt (`bool`, *optional*, defaults to `"True"`)inpipeline_controlnet_sd_xl.py:222where the signature isbool = True. That is a default-valueformatting issue rather than a wrong type, so it does not belong in the same PR. Happy to follow up
per pipeline family or as one batch — tell me which you prefer.
🤖 Written with Claude Code. All three sites were read in context and
each type was confirmed against the code that consumes the value.