Transfer ArrayViews from package to Base

[andreasnoack]

This is an updated version of #5556. I have copied the content of ArrayViews.jl to Base and committed with @lindahua as author. Are you okay with this @lindahua?

[IainNZ]

Factoid: ArrayViews.jl is required directly or indirectly by 75 other packages:

Top 10 packages by number of packages that depend on them:
URIParser          85
SHA                84
BinDeps            83
ArrayViews         75
JSON               66
StatsBase          65
Homebrew           54
Zlib               46
URLParse           39
Reexport           38

[lindahua]

I have no problem with this.

[jakebolewski]

Poking around with this, when building the system image we need to provide convert functions for ArrayViews -> Arrays. However this gives a MethodError as dispatch is calling jl_f_convert_default instead of the convert methods defined in arrayviews.jl. I'm still trying to wrap my head around the system image build process, @vtjnash any ideas?

[vtjnash]

these methods (since abstract), should generally not be assuming that a.arr is valid, but instead should call parent(a)

[vtjnash]

@jakebolewski either you've managed to cache an old version of convert, or perhaps have typed it too strongly, so it didn't apply when it should have. i can't say too much without seeing the code

[staticfloat]

@IainNZ I love that since SHA snuck in under BinDeps it immediately rocketed to the top. XD

[jakebolewski]

Typing it too strongly was my guess. Are you referencing the only line in inference.jl where you throw a method error? That doesn't seem to be the case but I could be wrong about that. I've pushed the current state to https://github.com/JuliaLang/julia/compare/jcb/arrayview branch, it should fail in file.jl.

[vtjnash]

much too strongly it looks like. almost all of those methods should be of the form:
fname{A<:Array}(::Type{A}, ...)
to avoid forcing requirements on the presence / value of type parameters

[andreasnoack]

For some reason this keeps failing on Travis, but the tests have passed on four machines locally. This PR just adds the view function and the Comtiguous- and StridedViews. It doesn't change the behavior of getindex or removes SubArrays which I think are the next steps in the transition. Hence I think we can merge this PR very soon.

[StefanKarpinski]

+1 — I would say go for it and see if anyone yells.

[IainNZ]

Our Travis tests have been failing badly, I wouldn't put much stock in it.

[staticfloat]

Your linux failure is #8200 (I can type that issue number by heart now). Your mac failure looks like travis just gave up while waiting for output, but it made its way through an awful lot of tests, so I'd say go for it.

[quinnj]

How long are the arrayviews tests supposed to take? Not sure if mine have hung or what, but it's still running after a few minutes........

windows 8.1 64-bit.

[timholy]

I love the idea of merging ArrayViews, but you cannot remove SubArrays until there's a plan to more forward more generally---ArrayViews wrap only Arrays, and there's a whole world of AbstractArrays that are only handled by SubArray (and ArrayViewsAPL). I, for one, use views of AbstractArrays about 100x more frequently than views of plain Arrays.

See also #7941 (comment), and my followup below that.

[quinnj]

Not sure if it was causing the hang, but I needed to put using Base.ArrayViews and import Base.ArrayViews to get the tests to pass for me locally.

[andreasnoack]

@timholy I see. My priority was to push the new behavior for array indexing to master as soon as possible such that everyone can start adjusting their code. I hadn't really grasped the problem of removing SubArray. Would the following make sense?

• Merge this PR
• Open a new PR experimenting with changing getindex to use views for Arrays
• Leave SubArrays until staged functions land and then start substituting SubArrays with functionality from ArrayViewsAPL

The last step could possibly also change Contiguous- and StridedViews if necessary. I think the important thing is that people start to get used to get views when indexing arrays.

[andreasnoack]

@quinnj I think you are experiencing the same error as I saw on Travis. Could you try to checkout the latest version and run the tests again?

[timholy]

I'm fine with that plan. But FYI, what I'm working on ATM is generating a version of ArrayViewsAPL that doesn't rely on stagedfunctions. It's quite possible I'll have a preliminary version working by tomorrow. You could either go ahead with your plan, or wait and see what that looks like before making a decision.

[quinnj]

Close since #8501 was merged?