[WIP]: update to [email protected]

[trxcllnt]

Continuing from #38, this PR includes the following changes:

• implements Arrow column reflection as an Arrow TypeVisitor
• updates main.cpp to reflect Arrow 0.3.0's internal API changes

I marked this PR as WIP until I can get all the arrow tests passing. Compiling with PSP_DEBUG=1, this is the stack trace of the failures:

  ● perspective.js › ASMJS › Constructors › Arrow constructor

    TypeError: Cannot read property '0' of undefined
      
      at __emval_get_property (build/webpack:/build/wasm_sync/psp.js:4947:1)
          at dynCall_iii (wasm-function[4691]:17)
      at invoke_iii (build/webpack:/build/wasm_sync/psp.js:5612:1)
          at __ZNK10emscripten3valixIiEES0_RKT_ (wasm-function[239]:69)
          at __ZN5arrow14fill_col_validEN10emscripten3valENSt3__210shared_ptrIN11perspective8t_columnEEE (wasm-function[238]:93)
          at dynCall_vii (wasm-function[4664]:17)
      at invoke_vii (build/webpack:/build/wasm_sync/psp.js:5369:1)
          at __Z10_fill_dataNSt3__210shared_ptrIN11perspective7t_tableEEENS_6vectorINS_12basic_stringIcNS_11char_traitsIcEENS_9allocatorIcEEEENS8_ISA_EEEEN10emscripten3valENS4_INS1_7t_dtypeENS8_ISF_EEEEjb (wasm-function[260]:2047)
          at dynCall_viiiiii (wasm-function[4694]:24)
      at invoke_viiiiii (build/webpack:/build/wasm_sync/psp.js:5639:1)
          at __Z10make_tablejN10emscripten3valES0_S0_jNSt3__212basic_stringIcNS1_11char_traitsIcEENS1_9allocatorIcEEEEN11perspective7t_dtypeEb (wasm-function[268]:599)
          at dynCall_viiiiiiiii (wasm-function[4669]:30)
      at invoke_viiiiiiiii (build/webpack:/build/wasm_sync/psp.js:5414:1)
          at __ZN10emscripten8internal7InvokerINSt3__210shared_ptrIN11perspective7t_tableEEEJjNS_3valES7_S7_jNS2_12basic_stringIcNS2_11char_traitsIcEENS2_9allocatorIcEEEENS4_7t_dtypeEbEE6invokeEPFS6_jS7_S7_S7_jSD_SE_bEjPNS0_7_EM_VALESJ_SJ_jPNS0_11BindingTypeISD_EUt_ESE_b (wasm-function[859]:227)
          at dynCall_iiiiiiiiii (wasm-function[4671]:30)
      at dynCall_iiiiiiiiii_1 (eval at makeDynCaller (build/webpack:/build/wasm_sync/psp.js:2360:1), <anonymous>:4:12)
      at Object.make_table (eval at new_ (build/webpack:/build/wasm_sync/psp.js:2439:1), <anonymous>:15:10)
      at Object.table (build/webpack:/src/js/perspective.js:1253:30)

My wasm-foo is not strong, but I'm happy to continue work here with some guidance (@nmichaud?) from your team. Thanks!

p.s. I apologize on behalf of VSCode for all the white-space edits.

[trxcllnt]

Ah after reviewing, it seems only the tests that use the test-null.arrow file are failing (tests/constructors.js and tests/internal.js). The tests in test/updates.js are passing.

[nmichaud]

@trxcllnt This is great, thanks! I'm currently traveling but will take a look at where it's failing within the webasm. Unfortunately the sourcemap support for webasm is lacking in Chrome, so I've had to revert to printf style debugging when there are issues in the integration wrapper.

[nmichaud]

@trxcllnt OK, found the issue. It looks like DictionaryVectors aren't accessing their nullBitmaps from the indices array - so this line is failing: https://github.com/jpmorganchase/perspective/pull/55/files#diff-2a6d0602bc7b53fd50c4a37a3f1ec83aR461.

Looking at the arrowjs code, it looks like the nullBitmap attribute on Vector is automatically proxied onto the data attribute (https://github.com/apache/arrow/blob/master/js/src/vector.ts#L61), while for DictionaryVectors this should be pointing to the indices object. The nullCount, however, seems to be correct. I'm having a little trouble debugging getters in chrome, so I'm not able to see where that attribute is being accessed from.

Edit - it looks like the nullCount is being proxied from indices array on the DictionaryData object - https://github.com/apache/arrow/blob/master/js/src/data.ts#L160, so probably just need to do the same for the nullBitmap.

[trxcllnt]

@nmichaud ah nice catch! We should add a nullBitmap getter to DictionaryData. It may take a few days to turnaround a new ASF release vote, so in the meantime can I special-case the nullBitmap lookup for DictionaryVectors for this PR?

[nmichaud]

@trxcllnt There's no rush to get this merged. I'd rather not add special casing to get around a bug in an upstream library, unless you think it will be a while before there is a new arrowjs release with the fix.

[trxcllnt]

@nmichaud sounds good. I opened https://issues.apache.org/jira/browse/ARROW-2214 and apache/arrow#1659. We should be able to get a 0.3.1 release out in the next few days.