Re: [DISCUSS] Result vs Status

[Micah Kornfield <em...@gmail.com>]

This seems reasonable to me.  Give the impact of the API changes I think it
might be worth keeping around for ~3 releases, but I think we are generally
slow to delete deprecated APIs anyways.

Any other thoughts on this?  i can try to open up some tracking JIRAs for
the work involved.

On Wed, Oct 30, 2019 at 1:25 PM Wes McKinney <we...@gmail.com> wrote:

> Returning to this discussion.
>
> Here is my position on the matter since this was brought up on the
> sync call today
>
> * For internal / non-public and pseudo-non-public APIs that have
> return/out values
>   - Use Result or Status at discretion of the developer, but Result<T>
> is preferable
>
> * For new public APIs with return/out values
>   - Prefer Result<T> unless a Status-based API seems definitely less
> awkward in real world use. I have to say that I'm skeptical about the
> relative usability of std::tuple outputs and don't think we should
> force the use of Result<T> for technical purity reasons
>
> * For existing Status APIs with return values
>   - Incrementally add Result<T> APIs and deprecate Status-based APIs.
> Maintain deprecated Status APIs for ~2 major releases
>
> On Thu, Oct 24, 2019 at 5:16 PM Omer F. Ozarslan <om...@utdallas.edu>
> wrote:
> >
> > Hi Micah,
> >
> > You're right. Quite possible that clang-query counted same function
> > separately for each include in each file. (I was iterating each file
> > separately, but providing all of them at once didn't change the result
> > either.)
> >
> > It's cool and wrong, so not very useful apparently. :-)
> >
> > Best,
> > Omer
> >
> > On Thu, Oct 24, 2019 at 4:51 PM Micah Kornfield <em...@gmail.com>
> wrote:
> > >
> > > Hi Omer,
> > > I think this is really cool.  It is quite possible it was
> underestimated (I agree about line lengths), but I think the clang query is
> double counting somehow.
> > >
> > > For instance:
> > >
> > > "grep -r Status *" only returns ~9000 results in total for me.
> > >
> > > Similarly using grep for "FinishTyped" returns 18 results for me.
> Searching through the log that you linked seems to return 450 (for "Status
> FinishTyped").
> > >
> > > It is quite possible, I'm doing something naive with grep.
> > >
> > > Thanks,
> > > Micah
> > >
> > > On Thu, Oct 24, 2019 at 2:41 PM Omer F. Ozarslan <om...@utdallas.edu>
> wrote:
> > >>
> > >> Forgot to mention most of those lines are longer than line width while
> > >> out is usually (always?) last parameter, so probably that's why grep
> > >> possibly underestimates their number.
> > >>
> > >> On Thu, Oct 24, 2019 at 4:33 PM Omer F. Ozarslan <om...@utdallas.edu>
> wrote:
> > >> >
> > >> > Hi,
> > >> >
> > >> > I don't have much experience on customized clang-tidy plugins, but
> > >> > this might be a good use case for such a plugin from what I read
> here
> > >> > and there (frankly this was a good excuse for me to have a look at
> > >> > clang tooling as well). I wanted to ensure it isn't obviously
> overkill
> > >> > before this suggestion: Running a clang query which lists functions
> > >> > returning `arrow::Status` and taking a pointer parameter named `out`
> > >> > showed that there are 13947 such functions in `cpp/src/**/*.h`. [1]
> > >> >
> > >> > I checked logs and it seemed legitimate to me, but please check it
> in
> > >> > case I missed something. If that's the case, it might be tedious to
> do
> > >> > this work manually.
> > >> >
> > >> > [1]: https://gist.github.com/ozars/ecbb1b8acd4a57ba4721c1965f83f342
> > >> > (Note that the log file is shown as truncated by github after ~30k
> > >> > lines)
> > >> >
> > >> > Best,
> > >> > Omer
> > >> >
> > >> >
> > >> >
> > >> > On Wed, Oct 23, 2019 at 9:23 PM Micah Kornfield <
> emkornfield@gmail.com> wrote:
> > >> > >
> > >> > > OK, it sounds like people want Result<T> (at least in some
> circumstances).
> > >> > > Any thoughts on migrating old APIs and what to do for new APIs
> going
> > >> > > forward?
> > >> > >
> > >> > > A very rough approximation [1] yields the following counts by
> module:
> > >> > >
> > >> > >  853 arrow
> > >> > >
> > >> > >   17 gandiva
> > >> > >
> > >> > >   25 parquet
> > >> > >
> > >> > >   50 plasma
> > >> > >
> > >> > >
> > >> > >
> > >> > > [1] grep -r Status cpp/src/* |grep ".h:" | grep "\\*" |grep -v
> Accept |sed
> > >> > > s/:.*// | cut -f3 -d/ |sort
> > >> > >
> > >> > >
> > >> > > Thanks,
> > >> > >
> > >> > > Micah
> > >> > >
> > >> > >
> > >> > >
> > >> > > On Sat, Oct 19, 2019 at 7:50 PM Francois Saint-Jacques <
> > >> > > fsaintjacques@gmail.com> wrote:
> > >> > >
> > >> > > > As mentioned, Result<T> is an improvement for function which
> returns a
> > >> > > > single value, e.g. Make/Factory-like. My vote goes Result<T>
> for such
> > >> > > > case. For multiple return types, we have std::tuple like Antoine
> > >> > > > proposed.
> > >> > > >
> > >> > > > François
> > >> > > >
> > >> > > > On Fri, Oct 18, 2019 at 9:19 PM Antoine Pitrou <
> antoine@python.org> wrote:
> > >> > > > >
> > >> > > > >
> > >> > > > > Le 18/10/2019 à 20:58, Wes McKinney a écrit :
> > >> > > > > > I'm definitely uncomfortable with the idea of deprecating
> Status.
> > >> > > > > >
> > >> > > > > > We have a few kinds of functions that can fail:
> > >> > > > > >
> > >> > > > > > 1. Functions with no "out" arguments
> > >> > > > > > 2. Functions with one out argument
> > >> > > > > > 3. Functions with multiple out arguments
> > >> > > > > >
> > >> > > > > > IMHO functions in category 2 are the best candidates for
> utilizing
> > >> > > > > > Status. In some cases, Case 3 may be more usable
> Result-based, but it
> > >> > > > > > can also create more work (or confusion) on the part of the
> developer,
> > >> > > > > > either
> > >> > > > > >
> > >> > > > > > * The T in Result<T> has to be a struct-like value that
> transports
> > >> > > > > > multiple pieces of data
> > >> > > > >
> > >> > > > > The T can be a std::tuple though, so you need not necessarily
> define a
> > >> > > > > dedicated struct type for a single API's return value.
> > >> > > > >
> > >> > > > >  > Can't say I'm thrilled about having Result<void> or
> similar for Case
> > >> > > > >  > 1-type functions (if I'm understanding what would be the
> solution
> > >> > > > >  > there).
> > >> > > > >
> > >> > > > > Agreed.
> > >> > > > >
> > >> > > > > Regards
> > >> > > > >
> > >> > > > > Antoine.
> > >> > > >
>