[design] adding a validation decorator

[yiyixuxu]

We try to avoid using the kwargs argument at diffusers because it's error prone and makes our code less readable. However, there are some instances where a fixed signature might not be practical. For example, our default attention processor class AttnProcessor/AttnProcessor2.0 can now be used in conjunction with other attention processors such as IPAdapterProcessor. If we need to pass a new argument to IPAdapterProcessor, we will need to add it to the AttnProcessor's signature as well. It may be worthwhile to consider adding a validation decorator https://github.com/huggingface/huggingface_hub/blob/c528f78fa7404869f32a2b04eac0f9964a5c6f9e/src/huggingface_hub/utils/_validators.py#L46 for this use case

(Original discussion can be found here: #6573 (comment).)

[spezialspezial]

You are the second person saying that and it confuses me. Could someone explain? If I grep across the diffusers codebase there is a gazillion occurrences of kwargs usage, function signatures included. Why is that a precedent now?

Other than that, most languages not supporting variadic functions is another thing I don't get. If not one of these outsider dialects https://en.wikipedia.org/wiki/Variadic_function, what's in your list? Even if it's not supported in some language it's not clear, to me at least, whether this is an pedagogical move. If you are held hostage by big Visual Basic or the Rust user group enforcing their definition of "most languages" blink twice.

[Wauplin]

Hi @yiyixuxu, I just came across this post and I've read this thread as well. I agree with @patrickvonplaten that in general the **kwargs syntax makes it more difficult and error-prone to use an API.

About using a decorator to handle extra parameters I think it's fine (I'm the one that introduced validate_hf_hub_args in the first place in huggingface_hub) but should be thought thoroughly before implementing it. The risks with this hack are about the same as with kwargs: users can't know about the extra params, no early error if typo correction, etc. In the huggingface_hub example I think it's fine because it handles use_auth_token, a parameter that we want to definitely remove/un-document => the validator ensures backward compatibility while not promoting the legacy parameter in the methods signatures.

That being said, I don't exactly know what is the use case here. IMO obfuscation of legacy parameters is a legit use case but **kwargs can have other purposes. So just a warning here, no strong opinion on the specific use case discussed here :)

[spezialspezial]

Maybe we're just talking about different things. Like you said, you don't know what it's about but you agree. How would you support processors with varying, unknown arguments? How do you validate if you don't know ahead of time what to validate for? In this case kwargs.get already validates what is of interest, throws a well known and easily recognizable error, does the minimal amount of work necessary and is likely highly optimized across interpreters. This function is literally called a 1000 times per generation. Not too bad, not nothing either.

Everyone likes readability but does it really trump capabilities? Almost every main() in the world disagrees. It's somewhat getting into "But would you think of the children" territory. Why would you hate children, err, readability?

This issue seems to have a surprising huge push back. In the end I already worked around the issue and I don't know why I'm still invested. Sorry for that.

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.

[sayakpaul]

@yiyixuxu is this relevant?

[github-actions]

This issue has been automatically marked as stale because it has not had recent activity. If you think this still needs to be addressed please comment on this thread.

Please note that issues that do not follow the contributing guidelines are likely to be ignored.