kou commented on code in PR #46926:
URL: https://github.com/apache/arrow/pull/46926#discussion_r2720468498
##########
c_glib/arrow-glib/compute.cpp:
##########
@@ -4334,8 +4402,8 @@ garrow_rank_options_set_property(GObject *object,
switch (prop_id) {
case PROP_RANK_OPTIONS_NULL_PLACEMENT:
- options->null_placement =
- static_cast<arrow::compute::NullPlacement>(g_value_get_enum(value));
+ options->null_placement = garrow_optional_null_placement_to_raw(
+ static_cast<GArrowOptionalNullPlacement>(g_value_get_enum(value)));
Review Comment:
```suggestion
{
auto null_placement =
static_cast<GArrowOptionalNullPlacement>(g_value_get_enum(value));
if (null_placement == GARROW_OPTIONAL_NULL_PLACEMENT_UNSPECIFIED) {
options->null_placement = std::nullopt;
} else {
options->null_placement = null_placement
}
}
```
##########
c_glib/arrow-glib/compute.h:
##########
@@ -508,6 +508,28 @@ typedef enum /*<prefix=GARROW_NULL_PLACEMENT_>*/ {
GARROW_NULL_PLACEMENT_AT_END,
} GArrowNullPlacement;
+/**
+ * GArrowOptionalNullPlacement:
+ * @GARROW_OPTIONAL_NULL_PLACEMENT_AT_START:
+ * Place nulls and NaNs before any non-null values.
+ * NaNs will come after nulls.
+ * @GARROW_OPTIONAL_NULL_PLACEMENT_AT_END:
+ * Place nulls and NaNs after any non-null values.
+ * NaNs will come before nulls.
+ * @GARROW_OPTIONAL_NULL_PLACEMENT_UNSET:
+ * Do not specify null placement.
+ * Null placement should instead
+ *
+ * They are corresponding to `std::optional<arrow::compute::NullPlacement>`
values.
+ *
+ * Since: 12.0.0
Review Comment:
```suggestion
* Since: 24.0.0
```
##########
c_glib/arrow-glib/compute.h:
##########
@@ -508,6 +508,28 @@ typedef enum /*<prefix=GARROW_NULL_PLACEMENT_>*/ {
GARROW_NULL_PLACEMENT_AT_END,
} GArrowNullPlacement;
+/**
+ * GArrowOptionalNullPlacement:
+ * @GARROW_OPTIONAL_NULL_PLACEMENT_AT_START:
+ * Place nulls and NaNs before any non-null values.
+ * NaNs will come after nulls.
+ * @GARROW_OPTIONAL_NULL_PLACEMENT_AT_END:
+ * Place nulls and NaNs after any non-null values.
+ * NaNs will come before nulls.
+ * @GARROW_OPTIONAL_NULL_PLACEMENT_UNSET:
Review Comment:
Could you use `UNSPECIFIED` not `UNSET` like we did in
https://github.com/apache/arrow/pull/48489 ?
##########
c_glib/arrow-glib/compute.cpp:
##########
@@ -4357,7 +4425,8 @@ garrow_rank_options_get_property(GObject *object,
switch (prop_id) {
case PROP_RANK_OPTIONS_NULL_PLACEMENT:
- g_value_set_enum(value,
static_cast<GArrowNullPlacement>(options->null_placement));
+ g_value_set_enum(value,
+
garrow_optional_null_placement_from_raw(options->null_placement));
Review Comment:
```suggestion
if (options->null_placement.has_value()) {
g_value_set_enum(value, *options->null_placement);
} else {
g_value_set_enum(value, GARROW_OPTIONAL_NULL_PLACEMENT_UNSPECIFIED);
}
```
##########
c_glib/arrow-glib/compute.cpp:
##########
@@ -3032,25 +3041,50 @@ garrow_sort_key_class_init(GArrowSortKeyClass *klass)
0,
static_cast<GParamFlags>(G_PARAM_READWRITE | G_PARAM_CONSTRUCT_ONLY));
g_object_class_install_property(gobject_class, PROP_SORT_KEY_ORDER, spec);
+
+ /**
+ * GArrowSortKey::null-placement:
+ *
+ * Whether nulls and NaNs are placed at the start or at the end.
+ *
+ * Since: 15.0.0
Review Comment:
```suggestion
* Since: 24.0.0
```
--
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]
