Create writer with arrow::ipc::IPCWriteOptions

[askoa]

Which issue does this PR close?

Closes #4708

[alamb]

Thank you @askoa

I wonder if it might be worth adding some test to ensure this API is not accidentally broken / removed as part of some future refactoring

[askoa]

I wonder if it might be worth adding some test to ensure this API is not accidentally broken / removed as part of some future refactoring

The API dos not add new functionality. It provides a bridge to access the compression function in arrow-rs. I don't think 'someone might remove it' is a good reason to add test. The person removing the code might also remove the test. If anyone ends up accidentally removing it, then the integration with arrow-ballista will fail. I don't see a need to add new test for this.

[ursabot]

Benchmark runs are scheduled for baseline = 01d00fd and contender = 9331ee3. 9331ee3 is a master commit associated with this PR. Results will be available as each benchmark for each run completes.
Conbench compare runs links:
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ec2-t3-xlarge-us-east-2] ec2-t3-xlarge-us-east-2
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on test-mac-arm] test-mac-arm
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-i9-9960x] ursa-i9-9960x
[Skipped ⚠️ Benchmarking of arrow-datafusion-commits is not supported on ursa-thinkcentre-m75q] ursa-thinkcentre-m75q
Buildkite builds:
Supported benchmarks:
ec2-t3-xlarge-us-east-2: Supported benchmark langs: Python, R. Runs only benchmarks with cloud = True
test-mac-arm: Supported benchmark langs: C++, Python, R
ursa-i9-9960x: Supported benchmark langs: Python, R, JavaScript
ursa-thinkcentre-m75q: Supported benchmark langs: C++, Java

[Dandandan]

I wonder if it might be worth adding some test to ensure this API is not accidentally broken / removed as part of some future refactoring

The API dos not add new functionality. It provides a bridge to access the compression function in arrow-rs. I don't think 'someone might remove it' is a good reason to add test. The person removing the code might also remove the test. If anyone ends up accidentally removing it, then the integration with arrow-ballista will fail. I don't see a need to add new test for this.

Thank you very mucht for adding this feature, no worries for the test!

In some cases, features might be lost by someone moving / refactoring the code as part of another change - this happened quite a few times before in arrow-rs / arrow-datafusion! Tests covering those code paths help avoiding issues like that. In ballista this might be only discovered once the version is updated (which may be only after a new version is released).

[askoa]

In some cases, features might be lost by someone moving / refactoring the code as part of another change - this happened quite a few times before in arrow-rs / arrow-datafusion! Tests covering those code paths help avoiding issues like that. In ballista this might be only discovered once the version is updated (which may be only after a new version is released).

I don't know the scenarios you are referring to. But during those refactoring the person might remove the test along with the code. So, in my opinion, catching when arrow-ballista consumes the changes is the best way to catch such issues rather than adding tests.

[alamb]

But during those refactoring the person might remove the test along with the code

This is definitely true. Tests such as @Dandandan and I are referring to do not guarantee code isn't broken.

However, I for one, when reviewing PRs look quite carefully at the tests that are changed / modified so having them act as a "second check"

I think it is fine for this PR to not have added tests, but I do think they serve an important purpose which is why I am belaboring this point