Fix float grouping

[bkamins]

Fixes #2790

[nalimilan]

Woops. Though maybe it would be worth checking whether the data contains -0.0, and use the fast path if not? Is there also a problem with NaN and Inf?

[bkamins]

Is there also a problem with NaN and Inf?

No, it is OK, because:

julia> isinteger(Inf)
false

julia> isinteger(NaN)
false

We could add a check against -0.0. However, I feel that doing groupby floats is rare (and in general discouraged) therefore I thought we can skip the fast path in this case.

If you would prefer to add special handling of -0.0 instead please let me know and I can add this instead of what I propose now.

[nalimilan]

As you prefer, but I tend to think that it's good to support the fast path in as many cases as possible unless that's hard to do. For example, R uses floats by default when reading CSV files, so people could end up with float columns with only integers instead of Integer columns when transferring from or loading Arrow files written in R. That may even end up in a blog post comparing DataFrames.jl's performance against R. It's hard to anticipate what weird things people might do...

[bkamins]

As you prefer, but I tend to think that it's good to support the fast path in as many cases as possible unless that's hard to do.

I have implemented it and added tests showing the consequences. Let us decide which approach is better.

[bkamins]

I will finalize this PR with the current design (treat float as integer if possible). After thinking I think it is less breaking, as we just have to add only the following condition to the current rules:

-0.0 is considered not to be an integer

(so doing it this way is less breaking)

[bkamins]

@nalimilan - I have made the requested cleanups. This should be good to be merged. Thank you!

[bkamins]

Thank you!