Skip to content
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

Fix a case of advance indexing if the list of indices is not sorted. #96

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

boazmohar
Copy link
Contributor

  • This now evaluates to true:
x = arange(3*3*4).reshape((3, 3, 4))
b = array(x, sc, axis=0)
assert allclose(b[[1, 0], [0, 1], [0, 2]].toarray(), x[[1, 0], [0, 1], [0, 2]])
  • Mixed indexing already handled this case. I added more tests.
  • Still can't handle sorting keys with dim > 1:
    This test passes:
b = array(x, sc, axis=(0,1))
assert allclose(b[[0, 1], [0, 1], [0, 2]].toarray(), x[[0, 1], [0, 1], [0, 2]])

This will not:

b = array(x, sc, axis=(0,1))
assert allclose(b[[1, 0], [0, 1], [0, 2]].toarray(), x[[1, 0], [0, 1], [0, 2]])

@boazmohar
Copy link
Contributor Author

@jwittenbach Can you go over this?
Thanks!

@jwittenbach
Copy link
Contributor

jwittenbach commented Feb 21, 2017

@boazmohar sorry for taking so long to get to this. It's looking good to me.

Do we have an open issue that it fixes (no worries if we don't)? I'm assuming that, without this fix, the test you wrote would fail when the indices are not ordered, correct? If so, is this just because it did not set the _sorted property correctly?

@boazmohar
Copy link
Contributor Author

@jwittenbach no there is no open issue, and it is hard for me to remeber now but you also told me to get the order of the keys and not assume they are sorted:

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.

None yet

2 participants