Update on Enum.split

[QuentinDanjou]

Split was allowing values like 2.0, and higher but throwing on 1.0, 0.0, and -1.0 which was not coherent. Even it does not make any sense to give floating values, it feels weird that it allows some of them and not others.
This fix now allow these values.

[josevalim]

Good catch @QuentinDanjou! I believe it would be better though to change each clause of the original split function: https://github.com/elixir-lang/elixir/pull/9436/files#diff-6881431a92cd4e3ea0de82bf2338f8eaR2457

Can you please change your PR accordingly and send a test? Thank you!

[josevalim]

The update looks good. Now we only need a test and we should be good to go. The readme has instructions on how to run tests but feel free to ask if you have any questions!

[QuentinDanjou]

@josevalim Sorry I didn't get if you mean adding a test to the test suite that test this behavior or simply run the test locally as explained in the readme.

Here is the specific test on my local computer:

> bin/elixirc lib/elixir/lib/enum.ex -o lib/elixir/ebin 
> bin/elixir lib/elixir/test/elixir/enum_test.exs  
Excluding tags: [windows: true]

..........................................................................................................................................................................................................................................................................................................................................................

Finished in 1.5 seconds (1.3s on load, 0.1s on tests)
183 doctests, 163 tests, 0 failures

Randomized with seed 83240

[josevalim]

I meant adding a test. :)

[fskinner]

I'm having trouble to understand how this PR allows those values since is_integer(count) will always return false when the count variable is set to -1.0, 0.0 or 1.0.

It seems this fix does not allow this values, it removes every float value instead. Am I right? Or am I missing something?

[fskinner]

If that's the case, I believe adding the code below to the split/2 test would allow this PR to be merged.

    assert_raise FunctionClauseError, fn ->
      Enum.split([1, 2, 3], -2.0)
    end

    assert_raise FunctionClauseError, fn ->
      Enum.split([1, 2, 3], -1.0)
    end

    assert_raise FunctionClauseError, fn ->
      Enum.split([1, 2, 3], 0.0)
    end

    assert_raise FunctionClauseError, fn ->
      Enum.split([1, 2, 3], 1.0)
    end

    assert_raise FunctionClauseError, fn ->
      Enum.split([1, 2, 3], 2.0)
    end

    assert_raise FunctionClauseError, fn ->
      Enum.split([1, 2, 3], 3.0)
    end

I can't do it on the forked branch because of no access :)

[josevalim]

❤️ 💚 💙 💛 💜