Add indices convertion. #9237
Add indices convertion. #9237
Conversation
|
I'm not entirely convinced that this is a good fix. If Pandas has issues with using scalars to index them then it's their problem, and we shouldn't have all users who didn't need this functionality pay for |
|
Here's actually a slightly deep problem.
Here, tn1 is a tensor which is, kind of, not a slice object, and PyTorch (not Pandas) try to use it as an argument for getitem:
Error output for a '0-d tensor' (a common case for an index) makes this class's instances unfavorable as a getitem argument. |
|
cc @fmassa who originally suggested this strategy |
|
Note that I don't see any issue in merging this as is. It doesn't break backwards-compatibility, and fixes an issue with pandas (that we don't have control over). |
|
Idk, people can create random subsets multiple times and then it might matter performance wise. There’s really no good reason apart from the bug in pandas which should be reported there. See PEP 357 |
|
I still think that this doesn't matter, and we have already things like that in the Or we just say that it's a bug in pandas and do nothing about it ourselves :-) |
|
(speaking of which, we might probably want to add |
|
When pandas check index value: |
|
@floatn hum... I'm not sure that's the right place where the problem happens, because we are following the same behavior of numpy there. import numpy as np
from collections import Iterable
a = np.array(1)
print(isinstance(a, Iterable)) # True
iter(a)
# TypeError: iteration over a 0-d arraySo according to your reasoning it should also fail for numpy arrays. |
|
Now I'm convinced it is a pandas problem indeed. :) |
|
I'm ok with merging this, and avoids us having to index a 1d tensor (which is slow in 0.4) |
|
I am new to committing and NN as well. So, with lowest confidence:
-1 in the end. |
|
closed based on the concluded |
Fix #9211 .