Repository navigation
FIX: let pandas IndexInt64 work for boxplot - #13398
NelleV merged 3 commits into
Conversation
Sorry, something went wrong.
|
I think this fix is "OK" as a stopgap, but I think what really should happen is position be put through unit conversion (which I don't think it is). But I'll leave that for someone who actually uses boxplot to make sure it is done properly... Still think this should go in to make array-like things w/o units work OK. Happy to be told that someone's favourite units used to work, but break w/ this change. |
Sorry, something went wrong.
| positions = list(range(1, N + 1)) | ||
| elif len(positions) != N: | ||
| raise ValueError(datashape_message.format("positions")) | ||
| positions = np.array(positions) |
There was a problem hiding this comment.
This is a good fix for the case described in the ticket, but I think the code will continue raising obscure error when positions are not numbers. Can you add a check after this line about the dtype of the array being a number? Maybe something like:
if not isinstance(positions.dtype, numbers.Number):
raise TypeError("positions should be a iterable of dtype number, but found dtype %s" % blah blah blah.)When we end up supporting something else than numbers, this can be updated.
Sorry, something went wrong.
There was a problem hiding this comment.
I couldn't quite get that check to work; I had to check the first element. But if there is a better way, let me know.
Sorry, something went wrong.
|
Thanks! |
Sorry, something went wrong.
PR Summary
Closes #10022 by casting the list of positions to an array as suggested by @tacaswell
PR Checklist