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

BUG: Make dtype.descr error for out-of-order fields. #10562

Merged
merged 3 commits into from Feb 9, 2018

Conversation

Projects
None yet
3 participants
@charris
Copy link
Member

commented Feb 9, 2018

Backport of #10391.

cc @ahaldane since #6053 exposes this to user code more often.

Currently returns garbage, Now

>>> data = np.zeros(1, dtype=[('a', int), ('b', float)])
>>> data = data[['b','a']]
>>> data.dtype.descr
ValueError

@charris charris added this to the 1.14.1 release milestone Feb 9, 2018

y = self.data[['col2', 'col1']]
assert_equal(y.dtype.names, ('col2', 'col1'))
assert_raises(ValueError, lambda: y.dtype.descr)

This comment has been minimized.

Copy link
@ahaldane

ahaldane Feb 9, 2018

Member

These tests aren't valid in 1.14.1. I would replace the entire function with

dt = dtype({'names': ['a', 'b'], 
            'formats': ['i4', 'i4'], 
            'offsets': [4, 0]})
y = np.rec.fromrecords([(1, 2), (4, 5)], dtype=dt)
assert_raises(ValueError, lambda: y.dtype.descr)
@ahaldane

This comment has been minimized.

Copy link
Member

commented Feb 9, 2018

I am going to try out the online editor to put my fixup, let me know if it is annoying or doesn't work somehow.

@charris charris merged commit e651b50 into numpy:maintenance/1.14.x Feb 9, 2018

4 checks passed

ci/circleci Your tests passed on CircleCI!
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
lgtm analysis: Python No alert changes
Details

@charris charris deleted the charris:backport-10391 branch Feb 9, 2018

@charris

This comment has been minimized.

Copy link
Member Author

commented Feb 9, 2018

Thanks for the fix, Allan.

@eric-wieser

This comment has been minimized.

Copy link
Member

commented on 7dffcb2 Feb 9, 2018

Wait... does this fail on master already then?

@ahaldane

This comment has been minimized.

Copy link
Member

commented Feb 9, 2018

No, because master has the copy->view change. This is for the 1.14.x branch where that was reverted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.