Skip to content

Conversation

@zero323
Copy link
Member

@zero323 zero323 commented Oct 7, 2015

gettitem method throws IndexError exception when we try to access index after the last non-zero entry

from pyspark.mllib.linalg import Vectors
sv = Vectors.sparse(5, {1: 3})
sv[0]
## 0.0
sv[1]
## 3.0
sv[2]
## Traceback (most recent call last):
##   File "<stdin>", line 1, in <module>
##   File "/python/pyspark/mllib/linalg/__init__.py", line 734, in __getitem__
##     row_ind = inds[insert_index]
## IndexError: index out of bounds

… try to access index after the last non-zero entry.
@jkbradley
Copy link
Member

ok to test

@jkbradley
Copy link
Member

test this please

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May as well use inds here for clarity since that's what is used elsewhere

@jkbradley
Copy link
Member

LGTM

Ping @mengxr FYI. Also, which Spark versions are we patching?

@jkbradley
Copy link
Member

@zero323 Can you please add tags "[ML] [PYTHON]" to the title of this PR?

@SparkQA
Copy link

SparkQA commented Oct 8, 2015

Test build #1859 has finished for PR 9009 at commit d28a644.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@zero323 zero323 changed the title [SPARK-10973] __gettitem__ method throws IndexError exception when we… [SPARK-10973][ML][PYTHON] __gettitem__ method throws IndexError exception when we… Oct 8, 2015
@zero323
Copy link
Member Author

zero323 commented Oct 8, 2015

@jkbradley Done.

@jkbradley
Copy link
Member

Thanks! I'll merge this with master once tests pass.

Would you be able to send PRs against branch-1.3, branch-1.4, branch-1.5 in order to backport this to previous Spark versions? They can use the same JIRA number.

@SparkQA
Copy link

SparkQA commented Oct 8, 2015

Test build #1862 has finished for PR 9009 at commit a1898ee.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@jkbradley
Copy link
Member

merging with master

@zero323
Copy link
Member Author

zero323 commented Oct 10, 2015

@jkbradley #9062, #9063, #9064

@zero323
Copy link
Member Author

zero323 commented Oct 10, 2015

It should be possible to push this check before binary search: 8a695fe2c3344acd19279fcd539177426d436a02

@jkbradley
Copy link
Member

@zero323 Good point, that's better for sure. Do you mind preparing a patch for 1.6 for that? (I don't think it's necessary to backport it everywhere.)

@zero323
Copy link
Member Author

zero323 commented Oct 11, 2015

@jkbradley Sure, I can do it later this week. Should I open a new JIRA for that?

@jkbradley
Copy link
Member

Yes, please. Thanks!

asfgit pushed a commit that referenced this pull request Oct 12, 2015
…when asked for index after the last non-zero entry

See #9009 for details.

Author: zero323 <[email protected]>

Closes #9064 from zero323/SPARK-10973_1.5.
asfgit pushed a commit that referenced this pull request Oct 12, 2015
…when asked for index after the last non-zero entry

See #9009 for details.

Author: zero323 <[email protected]>

Closes #9063 from zero323/SPARK-10973_1.4.
asfgit pushed a commit that referenced this pull request Oct 13, 2015
…when asked for index after the last non-zero entry

See #9009 for details.

Author: zero323 <[email protected]>

Closes #9062 from zero323/SPARK-10973_1.3.
@zero323
Copy link
Member Author

zero323 commented Oct 13, 2015

@jkbradley I've created a JIRA and opened a PR.

@zero323 zero323 deleted the sparse_vector_index_error branch April 6, 2017 11:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants