Alright in light of this I'll rewrite that PR to a traits based approach. Thanks all
On Fri, Jul 23, 2021 at 9:12 AM Wes McKinney <wesmck...@gmail.com> wrote: > Like Antoine, I am not sure how comfortable I am with this the way it > is now. On one hand I see the benefits in the reduction of > boilerplate. On the other, making the code more "magical" through meta > constructs likely makes it less accessible to contributors. It also > makes it so that IDEs (Clion, VSCode, Visual Studio, or anything using > clang-powered LSP) won't be able to autocomplete enums (please correct > me if this is not true) — this alone to me is reason enough not to > make these changes (particularly the new way of writing switch / if > statements). > > On Thu, Jul 22, 2021 at 1:49 PM Felipe Aramburu <fel...@blazingdb.com> > wrote: > > > > It feels like not using regular enums would be optimizing more for the > > producers of the library than the users that will be consuming it. It > > could definitely reduce the burden of writing some boiler plate code but > > this means that everyone that wants to use Arrow has another concept they > > have to learn about in order to get started. > > > > ~felipe > > > > > > > > On Thu, Jul 22, 2021 at 11:52 AM Antoine Pitrou <anto...@python.org> > wrote: > > > > > > > > As I said on the PR, I'm worried that it will raise eyebrows or even > > > make people defiant about using our C++ APIs. Basically, users of a > > > language are accustomed to certain idioms and often wary of software > > > packages that redefine their own variants of core language > > > functionality. Even I find it slightly unsettling to write switch/case > > > statements based on pointer values of literal strings... > > > > > > This is a social problem (on the technical front, the change seems ok, > > > and I trust Benjamin that the underlying infrastructure is sound). > > > > > > This is why I would favor a mechanism where the reflection abilities > are > > > defined "on the side" (using e.g. a EnumTraits<...>), while the public > > > C++ APIs still use normal enums. > > > > > > Regards > > > > > > Antoine. > > > > > > > > > Le 22/07/2021 à 16:50, Benjamin Kietzman a écrit : > > > > Enumerations are frequently useful constructs, but carry some > overhead > > > > when used with bindings. Specifically mapping the integral values to > > > string > > > > representations and back and validation of both integral values and > > > strings > > > > requires boilerplate, frequently replicated across multiple > languages. > > > > > > > > https://github.com/apache/arrow/pull/10712 would replace some of the > > > enums > > > > defined in arrow/compute/api* with a metaprogramming construct which > > > > includes validation, stringification, and parsing from string. > > > > > > > > The question at hand is: is it too confusing for users of the C++ > API to > > > > replace familiar enums with anything else? > > > > > > > > *** > > > > > > > > For example, one replacement from that PR is > > > > > > > > /// - `emit_null`: a null in any input results in a null in the > > > output. > > > > /// - `skip`: nulls in inputs are skipped. > > > > /// - `replace`: nulls in inputs are replaced with the > replacement > > > > string. > > > > struct NullHandlingBehavior : public > EnumType<NullHandlingBehavior> > > > { > > > > using EnumType::EnumType; > > > > static constexpr EnumStrings<3> values() { return > {"emit_null", > > > > "skip", "replace"}; } > > > > static constexpr const char* name() { return > > > "NullHandlingBehavior"; } > > > > }; > > > > > > > > which replaces (not shown: deleted boilerplate for stringification, > > > > validation, etc in Python and C++): > > > > > > > > enum NullHandlingBehavior { > > > > /// A null in any input results in a null in the output. > > > > EMIT_NULL, > > > > /// Nulls in inputs are skipped. > > > > SKIP, > > > > /// Nulls in inputs are replaced with the replacement string. > > > > REPLACE, > > > > }; > > > > > > > > Values from an EnumType are constructed at compile time from a > string, > > > > so for example we don't lose the ability to write an expressive > switch > > > over > > > > their values: > > > > > > > > switch (*null_handling) { > > > > case *NullHandlingBehavior("emit_null"): > > > > // ... > > > > } > > > > > > > >
