-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
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
agg() swallows exceptions. #3238
Comments
Try raising |
I wasn't raising a TypeError myself; calling a function with an incorrect number of parameters raises a TypeError. Your solution is not acceptable. |
what exactly are you trying to accomplish? maybe put up a sample frame, function that is closer to what you are doing |
I have a custom aggregator function. This function calls another function; the parameter list was wrong. The error was ignored silently! It should not. |
@wesm ? |
agg tries to be flexible by applying the aggregator to the entire dataset Unfortunately, it's hard to predict what kind of Error will be raised by the user-provided Although I'm not very fond of this sort of magical behaviour for exactly the reason you If you have a solution that does not break existing code, let's discuss it. |
I like the current magic, but maybe there could be some kind of raise_exceptions flag (which defaults to False)... |
This is a debugging issue, it doesn't make sense to crudify signatures with "dev-time only" IMO. |
what about raising an exception if none of the items can be agg'ed while agg'ing item by item? It shouldn't break existing code (much) and it should solve the above case. |
Let me have a look at this soon and let you know what I think. |
There's one unit test that breaks by no longer suppressing the TypeError but on inspection I'd rather not support the behavior-- i.e. to be able to pass an aggregation function that ONLY works on the string columns (but fails on the numeric columns). The auto-exclusion of non-numeric data when doing, say, Any objections? |
I think that sounds right, grouper functions should be responsible for handling the data, |
agg() appears to swallow exceptions.
This was the source of a hard-to-find bug today.
Here's a program that replicates the problem.
The text was updated successfully, but these errors were encountered: