Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate signature as argument #2657

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

blythed
Copy link
Collaborator

@blythed blythed commented Nov 29, 2024

This PR disables the need to add signature='*args,**kwargs' or similar to a model.

@jieguangzhou
Copy link
Collaborator

jieguangzhou commented Dec 30, 2024

I thought about it and I’m not entirely sure if the concept of signature is really necessary for the model.

For an individual model, it actually doesn’t seem to need a signature.

The signature only comes into play when we use a trainer or a listener.

In fact, there’s a simpler logic:

For the keys used by trainer and listener:

key = "a" → "singleton" or "*args"
key = ("a", "b") → "*args"
key = {"a": "x", "b": "y"} → "**kwargs"
key = (("a", "b"), {"k": "x"}) → "*args, **kwargs"

This is a very straightforward mapping, simpler, more intuitive, and easier to maintain than using signature.

For a select and a key, we can uniquely and directly retrieve the corresponding formatted data.

WDTY?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[DEV-CON] Replace signature with auto-inferred signature (.predict) using meta-class
2 participants