pitrou commented on code in PR #36915:
URL: https://github.com/apache/arrow/pull/36915#discussion_r1287279072
##########
cpp/src/arrow/type_fwd.h:
##########
@@ -560,6 +560,10 @@ ARROW_EXPORT std::shared_ptr<DataType>
time64(TimeUnit::type unit);
ARROW_EXPORT std::shared_ptr<DataType> struct_(
const std::vector<std::shared_ptr<Field>>& fields);
+/// \brief Create a StructType instance by initializer_list
Review Comment:
```suggestion
/// \brief Create a StructType instance from (name, type) pairs
///
/// The struct's fields will all be nullable with no associated metadata.
```
##########
cpp/src/arrow/type_fwd.h:
##########
@@ -640,6 +654,17 @@ std::shared_ptr<Schema> schema(
std::vector<std::shared_ptr<Field>> fields, Endianness endianness,
std::shared_ptr<const KeyValueMetadata> metadata = NULLPTR);
+/// \brief Create a Schema instance
+///
+/// \param fields the schema's fields in the form of initializer_list
Review Comment:
```suggestion
/// \param fields (name, type) pairs of the schema's fields
```
##########
cpp/src/arrow/type_fwd.h:
##########
@@ -629,6 +633,16 @@ std::shared_ptr<Schema> schema(
std::vector<std::shared_ptr<Field>> fields,
std::shared_ptr<const KeyValueMetadata> metadata = NULLPTR);
+/// \brief Create a Schema instance
+///
+/// \param fields the schema's fields in the form of initializer_list
Review Comment:
```suggestion
/// \param fields (name, type) pairs of the schema's fields
```
##########
cpp/src/arrow/type.cc:
##########
@@ -2090,17 +2090,39 @@ Status SchemaBuilder::AreCompatible(const
std::vector<std::shared_ptr<Schema>>&
return Merge(schemas, policy).status();
}
+std::vector<std::shared_ptr<Field>> make_fields(
+ std::initializer_list<std::pair<std::string, std::shared_ptr<DataType>>>
init_list) {
Review Comment:
Can you please put this helper function in the anonymous namespace, and also
make the function name CamelCase (i.e. `MakeFields`)?
##########
cpp/src/arrow/type.cc:
##########
@@ -2090,17 +2090,39 @@ Status SchemaBuilder::AreCompatible(const
std::vector<std::shared_ptr<Schema>>&
return Merge(schemas, policy).status();
}
+std::vector<std::shared_ptr<Field>> make_fields(
+ std::initializer_list<std::pair<std::string, std::shared_ptr<DataType>>>
init_list) {
+ std::vector<std::shared_ptr<Field>> fields;
+ fields.reserve(init_list.size());
+ for (const auto& [name, type] : init_list) {
+ fields.push_back(field(name, type));
+ }
Review Comment:
Not sure this will work, but worth trying:
```suggestion
for (auto&& [name, type] : init_list) {
fields.push_back(field(std::move(name), std::move(type)));
}
```
@bkietz Thoughts?
##########
cpp/src/arrow/type_fwd.h:
##########
@@ -640,6 +654,17 @@ std::shared_ptr<Schema> schema(
std::vector<std::shared_ptr<Field>> fields, Endianness endianness,
std::shared_ptr<const KeyValueMetadata> metadata = NULLPTR);
+/// \brief Create a Schema instance
+///
Review Comment:
```suggestion
/// \brief Create a Schema instance from (name, type) pairs
///
/// The schema's fields will all be nullable with no associated metadata.
///
```
##########
cpp/src/parquet/arrow/schema.cc:
##########
@@ -839,7 +839,8 @@
std::function<std::shared_ptr<::arrow::DataType>(FieldVector)> GetNestedFactory(
switch (inferred_type.id()) {
case ::arrow::Type::STRUCT:
if (origin_type.id() == ::arrow::Type::STRUCT) {
- return ::arrow::struct_;
+ return static_cast<std::shared_ptr<::arrow::DataType> (*)(const
FieldVector&)>(
+ &::arrow::struct_);
Review Comment:
Or can use a lambda for clarity:
```suggestion
return [](FieldVector fields) {
return ::arrow::struct_(std::move(fields));
};
```
##########
cpp/src/arrow/type_fwd.h:
##########
@@ -629,6 +633,16 @@ std::shared_ptr<Schema> schema(
std::vector<std::shared_ptr<Field>> fields,
std::shared_ptr<const KeyValueMetadata> metadata = NULLPTR);
+/// \brief Create a Schema instance
+///
Review Comment:
```suggestion
/// \brief Create a Schema instance from (name, type) pairs
///
/// The schema's fields will all be nullable with no associated metadata.
///
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
