fix&test push/pull recarrays #2034

Merged
merged 4 commits into from Jun 25, 2012

Conversation

Projects
None yet
3 participants
Owner

minrk commented Jun 25, 2012

We were only pushing the dtype str for some reason,
which is totally wrong for recarrays.

closes #2033

man you're fast!! This only seems to happen on the pull. I can access things on the engines once they're pushed.

Owner

minrk commented Jun 25, 2012

This is less efficient that what we used to do for simple arrays, as sending the array is ~200B instead of ~3, but that's not very much for sending a real array, and it should be much more general.

Owner

minrk commented Jun 25, 2012

Test results for commit d8f2b33 merged into master
Platform: darwin

  • python2.6: Failed, log at https://gist.github.com/2986419 (libraries not available: cython matplotlib oct2py pygments pymongo qt rpy2 tornado wx wx.aui)
  • python2.7: OK (libraries not available: oct2py wx wx.aui)
  • python3.2: OK (libraries not available: cython matplotlib oct2py pymongo qt rpy2 wx wx.aui)

Not available for testing: python3.1

Owner

minrk commented Jun 25, 2012

Hm, unpickling dtypes doesn't seem to work in numpy 1.5. Need to work on it for a moment more.

Owner

minrk commented Jun 25, 2012

Okay, I think I got it for real this time. testing again...

Owner

fperez commented Jun 25, 2012

Code looks clean, running test_pr right now. Thanks for catching this one, @wkerzendorf, and as usual record-setting response time, @minrk :)

No need to thank me, @minrk did all the work (and really fast), well if now I only have to get my own code working ;-)

explicit dtype for str in recarray test
'str' would fail with zero-size in numpy < 1.6
Owner

fperez commented Jun 25, 2012

Test results for commit fc2de03 merged into master
Platform: linux2

Not available for testing: python2.6, python3.1

Owner

fperez commented Jun 25, 2012

rerunning test_pr in case b54e738 helps with the failures reported above...

Owner

minrk commented Jun 25, 2012

slow down - there's one more commit after b54.

Owner

fperez commented Jun 25, 2012

ok, restarted run with bdf7984.

Owner

minrk commented Jun 25, 2012

Hm, while I'm pretty sure bdf should work, I might take one more stab at sending not the dtype object itself.

Owner

minrk commented Jun 25, 2012

Nevermind - I'll stick with the full dtype object since I'm not super confident in my understanding of dtype objects. I'll take a stab at cleaning it up when I do my general serialization improvement pass for 0.14.

Owner

fperez commented Jun 25, 2012

Sounds good. Let's be conservative here, and we can ping Travis O. or Mark Wiebe in Austin for a bit of wisdom on possible improvements we could make here while remaining 100% correct.

Owner

minrk commented Jun 25, 2012

Test results for commit bdf7984 merged into master
Platform: darwin

  • python2.6: OK (libraries not available: cython matplotlib oct2py pygments pymongo qt rpy2 tornado wx wx.aui)
  • python2.7: OK (libraries not available: oct2py wx wx.aui)
  • python3.2: OK (libraries not available: cython matplotlib oct2py pymongo qt rpy2 wx wx.aui)

Not available for testing: python3.1

Owner

minrk commented Jun 25, 2012

It's relatively minor - with this change, we are sending a few hundred bytes of metadata when ~99% of cases are covered with less than five.

I'm pretty sure the right answer is:

dtype = array.dtype.descr if array.dtype.shape else array.dtype.str

But I don't want to make a change like that immediately before release.

Owner

fperez commented Jun 25, 2012

Test results for commit bdf7984 merged into master
Platform: linux2

  • python2.7: OK
  • python3.2: OK (libraries not available: cython matplotlib oct2py pygments pymongo rpy2 wx wx.aui)

Not available for testing: python2.6, python3.1

Owner

fperez commented Jun 25, 2012

Conservative but safe, and code looks clean; thanks for the added tests! Merging now, glad we caught this before 0.13!

fperez added a commit that referenced this pull request Jun 25, 2012

Merge pull request #2034 from minrk/recarray
fix&test push/pull recarrays: We were only pushing the dtype str for some reason, which is totally wrong for recarrays.

closes #2033

@fperez fperez merged commit feac402 into ipython:master Jun 25, 2012

Owner

fperez commented Jun 25, 2012

Perhaps we should leave an issue open on this, or is it 'in cache' enough that you want to just open the 0.14 PR with that change?

Owner

minrk commented Jun 25, 2012

I'll add a note to #1504, which is for the more general serialization adjustments (and for which I already have a months-old branch in-progress)

Owner

fperez commented Jun 25, 2012

Perfect, thanks.

@minrk minrk deleted the minrk:recarray branch Mar 31, 2014

mattvonrocketstein pushed a commit to mattvonrocketstein/ipython that referenced this pull request Nov 3, 2014

Merge pull request #2034 from minrk/recarray
fix&test push/pull recarrays: We were only pushing the dtype str for some reason, which is totally wrong for recarrays.

closes #2033
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment