Repository navigation
Conversation
Add @functools.wraps to the inner wrapper so it inherits __name__, __doc__, __module__ and the signature of the wrapped method. Without this, methods decorated with @apply_forward_hook (e.g. AutoencoderKL's encode and decode) lose their docstring and signature, so doc-builder cannot render them through the [[autodoc]] directive and they show up as "wrapper" with a generic (*args, **kwargs) signature on the live API docs. The same affects VQModel, ConsistencyDecoderVAE and the other autoencoders that use the decorator. Add regression tests asserting that the wrapped method exposes the right metadata (name, docstring, signature including default values, __wrapped__) and that the underlying offload hook still fires through the wrapper. Fixes huggingface#6575
9d4632e to
fbdae29
Compare
|
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. |
stevhliu
left a comment
There was a problem hiding this comment.
nice, thanks for this! imo seems a bit much to add a test for this simple fix, but i'll defer to @sayakpaul on whether we need to include this
|
Happy to defer on the test. I only added it because the #6575 breakage was silent: call-time behaviour was always fine, it only showed up in |
|
@sayakpaul I've updated the branch with main and trimmed the test down to a single minimal check (the wrapped method keeps its |
…ook-preserves-metadata
|
@sayakpaul I merged main again to fix the conflict in |
What does this PR do?
Fixes #6575
apply_forward_hookreturns an innerwrapperthat doesn't usefunctools.wraps. When accelerate 0.17 or newer is installed, every method it decorates (37 files insrc/diffusers/use it, mostly onencode/decode) loses its__name__and__doc__, has no__wrapped__, and reports the signature(self, *args, **kwargs).inspect.signature,help()and doc-builder all seewrapper, so with accelerate installed doc-builder'sautodocrendersAutoencoderKL.encode/decodeaswrapper(*args, **kwargs), with no docstring and a source link toaccelerate_utils.py.The hosted v0.39.0 docs still show them that way. The v0.40.0 and main docs render them correctly because #14131 moved the doc build to doc-builder's light install, which doesn't install accelerate, so the decorator returns the method unchanged there.
apply_forward_hookon main still doesn't usefunctools.wraps.This adds
@functools.wraps(method)towrapper. An acceleratepre_forwardhook still fires onencode/decodecalls, same as on main.With accelerate installed:
Coordination: in #6575, stevhliu linked the
@apply_forward_hookline aboveAutoencoderKL.decodeand wrote "I think the line here may be giving the doc-builder some hiccups when parsing the file, but I'm not entirely sure." On this PR, his review said "nice, thanks for this!" and left the call on whether to keep the test to sayakpaul.Tests
TestApplyForwardHookintests/others/test_utils.pychecks that a decorated function keeps its__name__and__doc__. It only catches a missingfunctools.wrapswhen accelerate is installed: without accelerate,apply_forward_hookreturns the function unchanged and the test passes with or without the fix. The "Fast tests for PRs" workflow installs accelerate and runstests/others.Run with Python 3.11, torch 2.8.0 and accelerate 1.15.0:
Claude was used to review this change.
Who can review?
@stevhliu @sayakpaul