pitrou commented on code in PR #46819:
URL: https://github.com/apache/arrow/pull/46819#discussion_r2161498834
##########
cpp/src/arrow/type.h:
##########
@@ -1014,7 +1014,9 @@ class ARROW_EXPORT DecimalType : public
FixedSizeBinaryType {
static Result<std::shared_ptr<DataType>> Make(Type::type type_id, int32_t
precision,
int32_t scale);
+ /// \brief Returns the precision
Review Comment:
Since we are adding docstrings, we should avoid just repeating what the
method name, otherwise the docstring is next to useless.
Random suggestion, but you can come up with something else.
```suggestion
/// \brief Return the decimal precision
///
/// The decimal precision is a positive integer smaller or equal
/// than the concrete decimal's type `kMaxPrecision`.
```
##########
cpp/src/arrow/type.h:
##########
@@ -1337,20 +1339,25 @@ class ARROW_EXPORT MapType : public ListType {
explicit MapType(std::shared_ptr<Field> value_field, bool keys_sorted =
false);
- // Validating constructor
+ /// Validating constructor
static Result<std::shared_ptr<DataType>> Make(std::shared_ptr<Field>
value_field,
bool keys_sorted = false);
+ /// \brief Returns the key field
std::shared_ptr<Field> key_field() const { return value_type()->field(0); }
+ /// \brief Returns the key type
std::shared_ptr<DataType> key_type() const { return key_field()->type(); }
+ /// \brief Returns the item field
std::shared_ptr<Field> item_field() const { return value_type()->field(1); }
+ /// \brief Returns the item type
std::shared_ptr<DataType> item_type() const { return item_field()->type(); }
std::string ToString(bool show_metadata = false) const override;
std::string name() const override { return "map"; }
+ /// \brief Returns the keys sorted
Review Comment:
This is grammatically incorrect, besides being poorly informative.
```suggestion
/// \brief Return the keys_sorted flag
///
/// If keys_sorted is true, then the keys for each map item should appear
/// in sorted order in the map data. If keys_sorted is false, then no
assumption
/// can be made on keys order.
```
##########
cpp/src/arrow/type.h:
##########
@@ -1337,20 +1339,25 @@ class ARROW_EXPORT MapType : public ListType {
explicit MapType(std::shared_ptr<Field> value_field, bool keys_sorted =
false);
- // Validating constructor
+ /// Validating constructor
Review Comment:
Please let's make this a real informative docstring too.
--
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]
