ARROW-2286: [C++/Python] Allow subscripting pyarrow.lib.StructValue #1943
Conversation
|
Thanks @kszucs. Can you rebase to fix the CI failures? |
|
@pitrou rebase done, but the builds are still failing |
|
Ok, so the Travis-CI failure is unrelated (we are getting npm errors intermittently: see ARROW-2509). However, the Windows compile errors are probably caused by this PR. I'll point out the issue in a comment. |
| if index < 0: | ||
| raise KeyError(key) | ||
|
|
||
| return pyarrow_wrap_array(self.ap.field(index))[self.index] |
There was a problem hiding this comment.
You are passing an int64_t to a function that takes a (32-bit) int, hence the warning about truncation which is turned into an error.
pitrou
left a comment
There was a problem hiding this comment.
See my comments below about the likely cause of the Windows failure (and some more minor comments).
cpp/src/arrow/type.h
Outdated
| std::shared_ptr<Field> GetChildByName(const std::string& name) const; | ||
|
|
||
| /// Returns -1 if name not found | ||
| int64_t GetChildIndex(const std::string& name) const; |
There was a problem hiding this comment.
Child indices are int everywhere else, not int64_t, so this function should return int as well.
| int64_t GetChildIndex(const std::string& name) const; | ||
|
|
||
| private: | ||
| mutable std::unordered_map<std::string, int> name_to_index_; |
There was a problem hiding this comment.
Add a comment that this is lazily initialized?
cpp/src/arrow/type-test.cc
Outdated
| std::shared_ptr<Field> result; | ||
|
|
||
| result = struct_type.GetChildByName("f1"); | ||
| ASSERT_TRUE(f1->Equals(result)); |
There was a problem hiding this comment.
You could actually check for pointer equality here, i.e. ASSERT_EQ(f1, result).
cpp/src/arrow/type-test.cc
Outdated
| ASSERT_TRUE(f3->Equals(result)); | ||
|
|
||
| result = struct_type.GetChildByName("not-found"); | ||
| ASSERT_TRUE(result == nullptr); |
There was a problem hiding this comment.
ASSERT_EQ will produce nicer diagnostics on failure.
python/pyarrow/tests/test_scalars.py
Outdated
| assert isinstance(set_from_array, set) | ||
| assert set_from_array == {1, 2} | ||
|
|
||
| def test_struct_array_subscripting(self): |
There was a problem hiding this comment.
Nit: call this test_struct_value_subscripting? You're not subscripting the array here.
No description provided.