Repository navigation
[fix] Add return types - #14874
[fix] Add return types#14874
Conversation
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
sayakpaul
left a comment
There was a problem hiding this comment.
Nice cool! Should we consider using a CI validation for it as well?
Should we do tuple[XModularPipeline, PipelineState] here, or [BlockState] for loop blocks?
Sure.
8ecea6b to
e8f6d93
Compare
|
@sayakpaul , i think the failing CI is related to huggingface/transformers#45783. i'll check it out further and let you know! |
| A method is only checked on the class where it's written, not on classes that inherit it. Any annotation passes, | ||
| including `-> None`. | ||
|
|
||
| Run from the repository root: | ||
|
|
||
| python utils/check_return_annotations.py | ||
| """ |
There was a problem hiding this comment.
Do we get a nice list of functions (including their paths in the codebase) that are missing the return type in the annotation?
There was a problem hiding this comment.
yeah it should print out the below if something is missing:
src/diffusers/models/embeddings.py:1297: TimestepEmbedding.forward has no return type annotation
src/diffusers/pipelines/flux/pipeline_flux.py:602: FluxPipeline.__call__ has no return type annotation
Found 2 methods without a return type annotation. Add one to each method above.
sayakpaul
left a comment
There was a problem hiding this comment.
Thanks for iterating. Just two additional comments.
|
@stevhliu feel free to resolve the conflicts and then merge :) |
e0a9609 to
8792f43
Compare
Adds standard return type annotation
-> XPipelineOutput | tupleto non-deprecated pipeline__call__andXOutput tuple[torch.Tensor]toModelMixin.forwardand submoduleforward.Skipped the modular pipeline blocks for now which currently say
-> PipelineState, but every block returns(components, state)tuple. Should we dotuple[XModularPipeline, PipelineState]here, or[BlockState]for loop blocks?