8000 Fix recfunctions.join_by bugs (issue #4216) by gabobert · Pull Request #4524 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

Fix recfunctions.join_by bugs (issue #4216) #4524

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Fix recfunctions.join_by bugs (issue #4216) #4524

wants to merge 1 commit into from

Conversation

@gabobert
Copy link
@gabobert gabobert commented Mar 20, 2014

This fixes the issue: "BUG: recfunctions.join_by fails if the keys are not in the same order #4216" and another bug.

@charris
Copy link
Member
charris commented Mar 23, 2014

@gabobert Universal test failure

FAIL: test_two_keys_two_vars (test_recfunctions.TestJoinBy2)

----------------------------------------------------------------------

Traceback (most recent call last):

File "/home/travis/virtualenv/python2.6/lib/python2.6/site-packages/numpy/lib/tests/test_recfunctions.py", line 679, in test_two_keys_two_vars

assert_equal(test.dtype, control.dtype)

File "/home/travis/virtualenv/python2.6/lib/python2.6/site-packages/numpy/ma/testutils.py", line 100, in assert_equal

raise AssertionError(msg)

AssertionError:

Items are not equal:

ACTUAL: dtype([('a', '<i8'), ('k', '<i8'), ('b1', '<i8'), ('b2', '<i8'), ('c1', '<i8'), ('c2', '<i8')])

DESIRED: dtype([('k', '<i8'), ('a', '<i8'), ('b1', '<i8'), ('b2', '<i8'), ('c1', '<i8'), ('c2', '<i8')])

@gabobert
Copy link
Author
gabobert commented Mar 24, 2014

Now that joining two arrays with differently ordered key-fields is supported, the order of the key-fields in the returned array should match the key argument of join_by in my opinion. So the test should be changed to reflect this.

@charris
Copy link
Member
charris commented Mar 24, 2014

@gabobert Then you should change the test to match what it should return ;) It would be a g 8000 ood idea to post this change to the list for discussion.

@charris
Copy link
Member
charris commented Jan 9, 2016

#4216 is still open, but seems to have been fixed somewhere along the line. @ahaldane Comment?

@ahaldane
Copy link
Member
ahaldane commented Jan 11, 2016

I just tested, and #4216 was fixed by #5480. Specifically, that PR allows casting between structured types with differently ordered fields, which fixes #4216. I'll go ahead and close that issue and this PR.

Also, pinging myself in #6053 to think about this case. (I rework behavior of re-ordered fields there).

@ahaldane ahaldane closed this Jan 11, 2016
@ahaldane ahaldane mentioned this pull request Jan 11, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

03 - Maintenance component: numpy.lib

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gabobert @charris @ahaldane @rgommers
0