ARROW-13089: [Python] Allow creating RecordBatch from Python dict #10854
Conversation
python/pyarrow/table.pxi
Outdated
| v = mapping[field.name] | ||
| except KeyError: | ||
| try: | ||
| v = mapping[tobytes(field.name)] |
There was a problem hiding this comment.
I'm not sure allowing bytes keys is useful.
There was a problem hiding this comment.
Apparently we do support that in Table.from_pydict (but I agree this doesn't seem needed ..)
There was a problem hiding this comment.
to bytes function was removed.
jorisvandenbossche
left a comment
jorisvandenbossche
left a comment
There was a problem hiding this comment.
@kharoc thanks for the PR!
Since the implementation is almost identical between Recordbatch and Table, I think we should try to share the code. Both classes don't share a base class at the moment, so maybe a helper function is the easiest (the helper function can return the arguments to be passed to RecordBatch/Table.from_arrays)
python/pyarrow/table.pxi
Outdated
| v = mapping[field.name] | ||
| except KeyError: | ||
| try: | ||
| v = mapping[tobytes(field.name)] |
There was a problem hiding this comment.
Apparently we do support that in Table.from_pydict (but I agree this doesn't seem needed ..)
python/pyarrow/tests/test_table.py
Outdated
| assert result.equals(expected) | ||
|
|
||
|
|
||
| def test_recordbatch_from_pydict(): |
There was a problem hiding this comment.
Can you move this next to the existing test for table (to keep the similar tests close to each other). With some parametrization, we might also be able to reduce the duplication (the actual test doesn't seem to rely on anything RecordBatch/Table specific)
There was a problem hiding this comment.
It's solved. Parametrization was added.
|
@kharoc can you also take a look at my non-inline comment (#10854 (review)) about avoiding duplication between the Table and RecordBatch implementation? |
… _from_pydict function
jorisvandenbossche
changed the title
jorisvandenbossche
left a comment
jorisvandenbossche
left a comment
There was a problem hiding this comment.
Thanks for the update, looks good!
3 participants
Create a from_pydict function in RecordBatch class.
Create unit test for from_pydict