Rework IorRaise impl to use EmptyValue, and add tests

[kyay10]

A secondary goal of this PR is to remove the complexity that was inside IorRaise.recover, and hence transition closer to an implementation that would work with contexts (with addError being provided as a separate context, hence allowing the normal recover method to do the job right).

[serras]

This seems like a great idea; however if we are going to accept this in Arrow 1.x, there should be no removals at the API level. And to be honest, I don't fully understand we cannot have the Atomic<Any?> be part of IorRaise as we have in the current implementation.

[nomisRev]

I think removing the override for raise is incorrect, but besides that this looks good to me!

[nomisRev]

No star imports.

[nomisRev]

I don't think we can remove the override for raise, right? Don't we otherwise combine the error when doing raise now, are we? 🤔

I think a test is missing here:

ior(String::plus) {
  Ior.Both("Hello", Unit).bind()
  raise("World")
} shouldBe Ior.Left("Hello World")

[kyay10]

I have the final combine call happening inside the ior builder itself. This is so that try catch can recover from the error, and also so that any races in calling raise is only won by the call that actually reaches the builder, which is the behaviour of the rest of the builders.
That test passes (after adding a missing space) so no issues there.

removing the raise override is also good because it makes transitioning to context receivers much easier.

[nomisRev]

And to be honest, I don't fully understand we cannot have the Atomic<Any?> be part of IorRaise as we have in the current implementation.

Was this changed?

[kyay10]

Was this changed?

@nomisRev in a previous commit, I had removed the State and combineError in IorRaise in favour of a simpler addError method because that's all that IorRaise seems to need access to. However, that was a binary-breaking change. I implemented a version of that idea in arrow-context if you'd like to see it. Here's the ior builder, and here's the extension methods that IorRaise previously had (note the decomposition into Raise and Accumulate).

[nomisRev]

Thanks for the update, I am going to review this again.

Here's the ior builder, and here's the extension methods that IorRaise previously had (note the decomposition into Raise and Accumulate).

Really nice work! 🙌