Skip to content

Use native sizes by default when parsing buffer format string#621

Merged
stevengj merged 4 commits intoJuliaPy:masterfrom
JobJob:fix-buf-dtype
Dec 10, 2018
Merged

Use native sizes by default when parsing buffer format string#621
stevengj merged 4 commits intoJuliaPy:masterfrom
JobJob:fix-buf-dtype

Conversation

@JobJob
Copy link
Copy Markdown
Contributor

@JobJob JobJob commented Dec 6, 2018

Ahh oops. So, apparently you're supposed to use native sizes by default when parsing the buffer protocol format strings. I did the opposite in #487.

Current master

using PyCall
py"""
import numpy as np
"""
nparr = py"np.array([1,1,1,1])"o
nparr["dtype"]  # PyObject dtype('int64')
convert(PyAny, nparr)

returns Int32[1,0,1,0] *
It should just be Int[1,1,1,1]

This actually caused me a bug in some relatively large distributed code, that I found very hard to track down. So karmically, pretty interesting.

At least it didn't make it to a release.

* The [1, 0, 1, 0] is also in part due to a bug in f_contiguous for 1d arrays when stride != sizeof(T) - I will post a PR with a fix for that next week, I need to make some other changes to get the tests right for that. This is the minor positive to come out of this.

@stevengj
Copy link
Copy Markdown
Member

stevengj commented Dec 8, 2018

What is the AppVeyor failure?

@JobJob
Copy link
Copy Markdown
Contributor Author

JobJob commented Dec 10, 2018

Just an fyi: with the appveyor failures before, it seems like the conda x64 environments might not have actually been 64-bit builds of Python. The tests were failing because the dtype of a np.array([1,2,...,10]) in python was int32, not int64, but in julia Int was Int64 on those environments.

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.

2 participants