Depend on ArrayInterface

[Tokazama]

This is an initial attempt to start getting rid of Requires for ArrayInterface.

[codecov]

Codecov Report

Merging #268 (b63cc5e) into master (05cae5c) will decrease coverage by 2.92%.
The diff coverage is 0.00%.

❗ Current head b63cc5e differs from pull request most recent head fc2fda4. Consider uploading reports for the commit fc2fda4 to get more accurate results

@@            Coverage Diff             @@
##           master     #268      +/-   ##
==========================================
- Coverage   96.51%   93.58%   -2.93%     
==========================================
  Files           5        5              
  Lines         459      468       +9     
==========================================
- Hits          443      438       -5     
- Misses         16       30      +14     

Impacted Files	Coverage Δ
src/OffsetArrays.jl	94.39% <0.00%> (-4.01%)	⬇️
src/axes.jl	98.73% <0.00%> (-1.27%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 05cae5c...fc2fda4. Read the comment docs.

[johnnychen94]

I'm not sure if this is the right place to make changes, just throw some concerns here:

• this package is "more lightweight": this PR increases the loading time from 0.05s to 0.20s. I'm not going to start a new debate on the "more lightweight" stuff but would just refer to Proposal for first class support of conditional dependencies in Pkg JuliaLang/Pkg.jl#1285 (comment).
• this package is a non-standard test dependency to Julia https://github.com/JuliaLang/julia/blob/master/test/testhelpers/OffsetArrays.jl. Introducing non-standard library dependency would make updating codes there very difficult.
• and, this package is deliberately maintained to keep 1.0 compatibility. Even though we have 1.6 as the next LTS, I don't see a clear reason to drop 1.0 support unless someone does the statistics and concludes that dropping 1.0 support is harmless.

Edit:

Introducing non-standard library dependency would make updating codes there very difficult.

If we can keep the codes in a separate file (say, src/arrayinterface.jl) as a plugin, it would be easier to handle the manual cherry-pick.

[Tokazama]

Part of start up time is due to Requires. We're trying to resolve that by getting rid of it now that ArrayInterface.jl is better established. I don't have a strong opinion on how this transition should be managed within OffsetArays.jl, but we do need to move the OffsetArrays code here at some point.

[johnnychen94]

Can we have a stub package ArrayInterfaceSubs as JuliaArrays/ArrayInterface.jl#211 (comment) proposed? Until JuliaLang/Pkg.jl#1285 is supported, this is the best option so far if we want to remove the Requires latency.

cc: @chriselrod

[Tokazama]

We've always planned on getting rid of Requires, so resolution of JuliaLang/Pkg.jl#1285 shouldn't make a difference.

[timholy]

I think we want ArrayInterface pretty broadly, and if it's going to be loaded by multiple packages then it won't have a direct impact on this package (it will already be loaded by something else).

[timholy]

@johnnychen94's concerns about potential divergence for testing Base are quite relevant. For 1.0 compatibility, I'd agree that it's a disadvantage until Julia 1.6 is officially declared LTS. Once that happens, I say we abandon any duty to maintain compat with 1.0.

[johnnychen94]

I think we want ArrayInterface pretty broadly, and if it's going to be loaded by multiple packages then it won't have a direct impact on this package (it will already be loaded by something else).

I think this is where we diverges. My understanding of ArrayInterface is that it is still experimental and if anything that is stable enough it should goes to Base first. Letting every other community to adopt ArrayInterface really has the tendency to diverges from the Base Array interface especially when ArrayInterface itself doesn't have a solid documentation on how other packages should coordinate with it.

I can understand that LoopVectorization is built on top of ArrayInterface to extract enough information during compilation time, but really it is not clear from a non-ArrayInterface-maintainer's perspective what is needed to support LoopVectorization's magic turbo. If there is a clear interface documentation on "doing steps 1, 2, 3 and you'll get your array type supported by LoopVectorization" and then I'll just do it myself for every array types I know of.

We've always planned on getting rid of Requires, so resolution of JuliaLang/Pkg.jl#1285 shouldn't make a difference.

It makes differences. JuliaLang/Pkg.jl#1285 kills the debates of "which package is more lightweight than the other" and gives a solution that both parties are happy with. This PR simply moves latencies from ArrayInterface to OffsetArrays because some people think ArrayInterface ecosystem is more important than the OffsetArray ecosystem. I'm fine on having this dependency since it can be negligible if multiple packages are loaded but I still want to point out that the final solution is JuliaLang/Pkg.jl#1285 instead of persuading other packages to take the latency burden.

Once that happens, I say we abandon any duty to maintain compat with 1.0.

I'm okay with this.

[Tokazama]

My understanding of ArrayInterface is that it is still experimental and if anything that is stable enough it should goes to Base first.

The parts I've been working into this PR are very mature and the most unstable part of the whole package is the indexing protocol (which needs upstream adoption and testing to really identify potential problems). Some of these things would be breaking changes for Base, but much easier to transition into Base if they were adopted and tested in the wild first.

The caveat to this is that the Julia community may want to just wait until Julia 2.0 and handle all the breaking changes at once.

EDIT:
I haven't had time yet to work out the test here, but the code I've implemented translates into support for getting axes and size if OffsetArray wraps an array with named dimensions and supports methods like known_length and known_size when a static array is wrapped.

[johnnychen94]

My main concern of this PR is that without any documentation or test coverage, future maintainers won't know how to deal with these codes if they don't happen to know the ArrayInterface internals.

Maybe you guys can first update the documentation in ArrayInterfaces? Then I can understand the motivation behind the changes more.

[johnnychen94]

This might be a trivial question, but what's the benefit of this?

[Tokazama]

The biggest benefit of ArrayInterface.axes is that it facilitates statically sized axes. This helps with arrays like ReshapedReinterpretArray where Base uses Int instead of StaticInt to define the axis length. I should clean up and document that code in ArrayInterface before completing this PR.

to_dims is described in the docs https://juliaarrays.github.io/ArrayInterface.jl/dev/#Dimensions. If there's anything there that is unclear I'd be happy to improve the docs.

EDIT: latest PR to ArrayInterface adds a bunch of examples the docs and cleanup related to axes. Once that goes through I'll make relevant changes here.