lidavidm commented on a change in pull request #11019:
URL: https://github.com/apache/arrow/pull/11019#discussion_r705398811
##########
File path: cpp/src/arrow/compute/api_vector.h
##########
@@ -120,20 +120,29 @@ class ARROW_EXPORT SortOptions : public FunctionOptions {
std::vector<SortKey> sort_keys;
};
-// \brief Selection algorithm. Default is `NonStableSelect` which uses a Heap
based
-// algorithm.
-enum class SelectKAlgorithm { NonStableSelect, StableSelect };
-
/// \brief SelectK options
class ARROW_EXPORT SelectKOptions : public FunctionOptions {
public:
- explicit SelectKOptions(int64_t k = -1, std::vector<SortKey> sort_keys = {},
- SelectKAlgorithm kind =
SelectKAlgorithm::NonStableSelect);
+ explicit SelectKOptions(int64_t k = -1, std::vector<SortKey> sort_keys = {});
constexpr static char const kTypeName[] = "SelectKOptions";
- static SelectKOptions Defaults() {
- return SelectKOptions{-1, {}, SelectKAlgorithm::NonStableSelect};
+ static SelectKOptions Defaults() { return SelectKOptions{-1, {}}; }
+
+ static SelectKOptions TopKDefault(int64_t k, std::vector<std::string>
key_names = {}) {
+ std::vector<SortKey> keys;
+ for (const auto& name : key_names)
+ keys.emplace_back(SortKey(name, SortOrder::Descending));
Review comment:
nit: please add braces for this sort of thing
##########
File path: cpp/src/arrow/compute/kernels/vector_sort.cc
##########
@@ -1826,7 +1823,7 @@ const FunctionDoc top_k_doc(
"at the end of the array.\n"
"For floating-point types, NaNs are considered greater than any\n"
"other non-null value, but smaller than null values."),
- {"input"}, "TopKOptions");
+ {"input"}, "SelectKOptions::TopKDefault");
Review comment:
Though, why do we still even have top_k_doc and bottom_k_doc?
##########
File path: cpp/src/arrow/compute/kernels/vector_sort.cc
##########
@@ -1826,7 +1823,7 @@ const FunctionDoc top_k_doc(
"at the end of the array.\n"
"For floating-point types, NaNs are considered greater than any\n"
"other non-null value, but smaller than null values."),
- {"input"}, "TopKOptions");
+ {"input"}, "SelectKOptions::TopKDefault");
Review comment:
Note the param here should be just the name of the options class, i.e.
"SelectKOptions". Python uses this for introspection.
--
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]
