bkietz commented on a change in pull request #7120:
URL: https://github.com/apache/arrow/pull/7120#discussion_r422353994



##########
File path: cpp/src/arrow/util/value_parsing.h
##########
@@ -484,6 +455,14 @@ static inline bool ParseHH_MM_SS(const char* s, 
std::chrono::duration<ts_type>*
 
 }  // namespace detail
 
+/// \brief Attempt to convert a string to the primitive type corresponding to
+/// an Arrow data type
+template <typename T, typename ParseContext = void>
+inline bool ParseValue(const char* s, size_t length, typename T::c_type* out,
+                       const ParseContext* ctx = NULLPTR) {
+  return detail::StringConverter<T>::Convert(s, length, out);
+}
+

Review comment:
       It's confusing when signatures for generics like this differ 
significantly, so yes: as I wrote it the possibility of a non-empty context 
does push the requirement of a context argument onto overloads of `ParseValue` 
which are context free.
   
   If you feel `ParseValue<TimestampType>()` has requirements which are 
sufficiently different from `ParseValue<T>()` that you think the signatures 
*should* be different, then why should they be in the same overload family? 
What value does `ParseValue<TimestampType>()` provide over 
`ParseTimestampISO8601()`?




----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


Reply via email to