BUG: Series.__getitem__ with downstream scalars #32684
Conversation
pandas/core/series.py
Outdated
| # check for is_list_like/slice instead of is_scalar to allow non-standard | ||
| # scalars through, e.g. cftime.datetime needed by xarray | ||
| # https://github.com/pydata/xarray/issues/3751 | ||
| key_is_scalar = not is_list_like(key) and not isinstance(key, slice) |
There was a problem hiding this comment.
this is really really sketch, why don't we just fix is_scalar?
also pls add a test if you can for this
There was a problem hiding this comment.
why don't we just fix is_scalar?
Depends on what we want is_scalar to mean. We could re-write is_scalar to just match this (actually more performant than what we have now), but it isn't really viable to add checks for any custom scalar that downstream libraries might implement.
Going to wait to hear from the downstream folks on if this actually solves their problem(s)
There was a problem hiding this comment.
I actually think is_scalar should just be this its much more generic and future proof
There was a problem hiding this comment.
hmm so changing is_scalar to just match this is breaking a bunch of tests bc is_scalar(some_lambda_func) is returning True
There was a problem hiding this comment.
right you might need some more exclusions, e.g. callabes
There was a problem hiding this comment.
so the trouble im facing ATM is that TimeGrouper is being recognized as a scalar
There was a problem hiding this comment.
The more I look at this the more skeptical I am of trying to amend is_scalar
There was a problem hiding this comment.
since we don't actually have a test case for this, am inclined to close.
There was a problem hiding this comment.
coming up with a test case isnt difficult, just need to settle on an approach.
jreback
added
the
Indexing
|
Thanks @jbrockmendel -- sorry I just saw this now. Yes, as written, this change indeed fixes the remaining issue for us, though I think @jreback's suggestion would work too. |
|
Updated with a better implementation, and a test that fails in master: |
|
much better, thanks. |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffcc @spencerclark I think this fixes a subset of the issues reported in pydata/xarray#3751, can you confirm?
@jorisvandenbossche IIRC geopandas scalars not being recognized by lib.is_scalar has caused some issues there; does this address any of those?