On DataAPI

[aplavin]

In GitLab by @jariji on Jul 4, 2023, 22:55

Currently FlexiJoins.innerjoin is using DataAPI.innerjoin which means it gives the DataFrames docs and not the FlexiJoins docs. I don't see much advantage of using DataAPI functions here. The main advantage seems to be that I don't need to qualify the name if both are in scope, but that doesn't seem worth giving up the semantic distinction imho. What do you think?

[aplavin]

Tbh, I didn't really think much about this decision - it just seemed natural to extend DataAPI's functions given that they exist (:

Maybe there's no real reason indeed. This DataAPI function extension only affects scenarios when there's another innerjoin imported from another package, for me this is basically "never". I remember a few times I used SplitApplyCombine together and got a clash, but the latter doesn't extend DataAPI anyway so it doesn't help.

Still, I think removing this is technically breaking, so I'm quite hesitant to perform this change unless there are compelling reasons, like actual issues arising.

it gives the DataFrames docs and not the FlexiJoins docs

Don't understand how DataFrames are related at all here? For me, ?innerjoin does give the correct FlexiJoins docstring, no matter if using FlexiJoins alone or also using DataAPI. The docstring is pretty short, but sparse documentation is another issue, I'm aware of it (:

[aplavin]

In GitLab by @jariji on Jul 5, 2023, 04:15

I can't reproduce the docs problem I mentioned, maybe I just missed it.

Still, I think removing this is technically breaking, so I'm quite hesitant to perform this change unless there are compelling reasons, like actual issues arising.

Perhaps if there's a breaking change someday, then.