paleolimbot commented on code in PR #258:
URL: https://github.com/apache/arrow-nanoarrow/pull/258#discussion_r1268079274
##########
dist/nanoarrow.h:
##########
Review Comment:
Can you revert the change to this file? I should add the hook to modify it
to pre-commit but right now it's updated via a nightly job (for better or
worse).
##########
src/nanoarrow/nanoarrow_types.h:
##########
@@ -656,6 +656,28 @@ struct ArrowArrayPrivateData {
int8_t union_type_id_is_child_index;
};
+/// \brief A representation of an interval.
+struct ArrowInterval {
+ /// \brief The type of interval being used
+ enum ArrowType unit;
+ int32_t months;
+ int32_t days;
+ int32_t ms;
+ int64_t ns;
+};
Review Comment:
Awesome!
I think this will need the fields documented for them to show up in the
generated documentation (and `\ingroup nanoarrow-utils` for it to show up in
the right place).
We have a lot of `ArrowType` variables that are named `type`...would that be
appropriate here?
##########
src/nanoarrow/nanoarrow_types.h:
##########
@@ -656,6 +656,28 @@ struct ArrowArrayPrivateData {
int8_t union_type_id_is_child_index;
};
+/// \brief A representation of an interval.
+struct ArrowInterval {
+ /// \brief The type of interval being used
+ enum ArrowType unit;
+ int32_t months;
+ int32_t days;
+ int32_t ms;
+ int64_t ns;
+};
+
+/// \brief Initialize an Interval with a given set of values
+/// \ingroup nanoarrow-utils
+static inline void ArrowIntervalInit(struct ArrowInterval* interval, enum
ArrowType unit,
+ int32_t months, int32_t days, int32_t ms,
+ int64_t ns) {
+ interval->unit = unit;
+ interval->months = months;
+ interval->days = days;
+ interval->ms = ms;
+ interval->ns = ns;
+}
Review Comment:
I think this can just zero-initialize and set `unit` and encourage direct
field access like `ArrowDecimal`? Most users won't use all the fields at once.
--
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]
