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

Should a function with **kwargs throw unexpected keyword argument ? #1749

Open
FVolral opened this issue Sep 9, 2023 · 3 comments
Open

Should a function with **kwargs throw unexpected keyword argument ? #1749

FVolral opened this issue Sep 9, 2023 · 3 comments
Labels
question Issues asking for help doing something

Comments

@FVolral
Copy link

FVolral commented Sep 9, 2023

hello, there is probably a small typo with tonnetz. If you use a bad argument, lets says n_fft, you will got
the error message TypeError: **chroma_cqt()** got an unexpected keyword argument 'n_fft' which is misleading. It smells the copypasta

@bmcfee
Copy link
Member

bmcfee commented Sep 9, 2023

I don't think I understand the problem here. You're providing an unexpected argument, and the error message says exactly that. What is misleading, or what would you expect it to say instead?

@lostanlen
Copy link
Contributor

I think that the misleading element, according to @FVolral, is that the unexpected keyword argument is passed from tonnetz to chroma_cqt, so the TypeError is thrown in the latter whereas the user called the former. So from my understanding, it's not at all a typo but a question about the way of handling exceptions when our functions have keyword arguments.

If my understanding is right, then I think that librosa is working as intended here, and consistently with packages like NumPy or Matplotlib. @FVolral try running plt.plot([0, 1, 2], unexpected='arg') and you'll see a similar behavior:

AttributeError: Line2D.set() got an unexpected keyword argument 'unexpected'

The exception is thrown in Line2D.set, not in plot.

@lostanlen lostanlen added the question Issues asking for help doing something label Sep 11, 2023
@lostanlen lostanlen changed the title typo Should a function with **kwargs throw unexpected keyword argument ? Sep 11, 2023
@bmcfee
Copy link
Member

bmcfee commented Sep 11, 2023

Ahh, I see. @lostanlen is correct - this is not copypasta, but instead an exception being raised one layer deeper in the call stack than the user might be expecting.

Since the underlying exception here is a TypeError, it would be impossible to wrap pass-through calls in a try/except block that would only trigger on unexpected keyword arguments.

Alternatively, we could imagine cooking up some machinery that would inspect the call signature of nested calls against the kwargs we're going to send through before calling, and raise an error earlier, but this seems like a pretty heavy-weight solution for a rather niche problem.

In general, I don't think there's too much we can do here, unless we want to go the route of explicitly registering all pass-through kwargs in the call signature. Given the complexity of the call signature already, I'm not inclined to go that route.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Issues asking for help doing something
Development

No branches or pull requests

3 participants