Plots.jl: Plots should implement the generalized Arrays interface, to be compatible with e.g. OffsetArrays

Created on 27 Jun 2017  路  17Comments  路  Source: JuliaPlots/Plots.jl

Plots doesn't work with OffsetArrays:

julia> plot(OffsetArray(rand(100), -50))
ERROR: size not supported for arrays with indices (-49:50,); see http://docs.julialang.org/en/latest/devdocs/offset-arrays/
enhancement priority

Most helpful comment

Plots.jl should use the recommended API for working with arrays

Definitely, I wasn't aware we weren't doing that. Let's put together a PR to address that and make it a priority.

All 17 comments

Can it get a type-recipe in a package?

It could have a type recipe in Plots, I think.

Ah, I thought they were in Base Julia now. The support should probably be in StatPlots, I wouldn't mind adding a dep on JuliaArrays. @ChrisRackauckas is right it would be better to put a recipe in OffsetArrays, but I don't think they'd want it.

@yakir12 , can you describe exactly how you'd expect them to work? e.g. would you pass arrays with different offsets, and how would that work?

The way I see it, an OffsetArray (say a vector) doesn't only contain information about the y-values but also about the x-values as well. So plotting such a vector should naturally plot each y-value in that vector at a specific x-value. When plotting a "normal" vector one assumes it should start at x=1 and stop at the length of that vector. With OffsetArrays, you know where it should start and stop depending on the offsets.

I'm trying to think though whether there are other additional circumstances where OffsetArrays could be utilized differently: interpolation, numerical integration, regression..? The only reason I'm rambling about that is that we might need/should start some larger initiative to standardize how all the packages deal with OffsetArrays (not just Plots)...

Oh @yakir12 , are you involved with OffsetArrays? That's great, so let me make an argument for putting this capability in the JuliaArrays package instead of e.g. StatPlots:

The idea of recipes is that all julia packages that define a new type should be able to define a plotting behaviour for that type, _without_ depending on a plotting package, potentially creating dependency conflicts etc.
All they require is a dependency on RecipesBase, which is 300 lines of code, has no dependencies, is rarely updated (and only with new julia versions), and doesn't put any functions in the workspace that conflict with other plotting packages. Having the recipe in the package that defines the type leads to much more seamless maintenance when types are updated, and ensures that type authors, not the authors of plotting packages, determine the behaviour of types. It also makes a standardization like what you talk about much easier.

A recipe that would do what you describe is exceedingly simple to program:

@recipe f(o::OffsetArray{T,1}) where T = (eachindex(o),o.parent)

This line of code is enough to allow you to do:

using OffsetArrays, Plots
plot(OffsetArray(rand(100), -50))

skaermbillede 2017-08-06 kl 16 38 43

It may be worth considering the options, though. When defined like this, you cannot plot two OffsetArrays against each other, because they already define an x and a y, and you can't do stuff like a histogram (what's it supposed to do with the x's?).

You could also throw away the offsets for plotting and treat them like any other array with values - and then perhaps define a special function for showing the values against the indices. E.g.

# a type recipe that just extracts the values from an OffsetArray
@recipe f{T<:OffsetArray}(::Type{T}, val::T) = val.parent

# the x,y functionality defined as a special function
@userplot OffsetPlot
@recipe f(o::OffsetPlot)
    isa(o.args[1], OffsetArray) || error("Only defined for OffsetArrays")
    eachindex(o.args[1]), o.args[1].parent
end

e.g.

oa = OffsetArray(rand(100), -50)
a = offsetplot(oa);
b = histogram(oa);
plot(a,b)

skaermbillede 2017-08-06 kl 16 50 06
I'd be happy to contribute a PR to OffsetArrays.

Let me start by saying that I'm unfortunately not involved with OffsetArrays, wish I was.

Not that it's worth anything to you, but heck, you managed to convince me! :star:

I guess the war on who should be compatible to whom depends on which one is more "basic", or which compatibility would result in the least amount of code (overall) and execution time. I probably share your opinion that "plotting" should be a basic requirement for a scientific programming language such as Julia. But any functionality that has to do with arrays and that will (if not yet) end up in base would probably be dimmed even more basic.

I love the idea of recipes (maybe your explanation should be blogged and posted on Discourse), and will add some to my own packages.

Anyways, sorry to waste your time, let me know if I can be of any help!

PS
Please ignore if this is trivial: but it might be really useful to see some benchmarking with and without those 300 lines of code for a bunch of packages -- just to see what the effect of adding those lines does to any package. If it will add ~0.01 seconds to the include SomePackage and then ~0 seconds to everything else, then the added benefit of being able to plot anything sounds like a pretty good deal to me!

Neither needs to support (or know about) the other, justbut Plots.jl should use the recommended API for working with arrays.

If it will add ~0.01 seconds to the include SomePackage and then ~0 seconds to everything else, then the added benefit of being able to plot anything sounds like a pretty good deal to me!

If it added a microsecond I would be confused file an issue in Julialang. It's RecipesBase.jl, not Plots.jl that would be the dependency. It defines I think 3 macros and 3 abstract types and that's it... plus it's precompiled.

As @timholy says, the true answer to this is just to respect the native indices of array types instead of assuming 1:length(A). That can be done with the tools in Base.

Plots.jl should use the recommended API for working with arrays

Definitely, I wasn't aware we weren't doing that. Let's put together a PR to address that and make it a priority.

I've created a PR (#999) from a shared branch, and assigned myself to it. I've also assigned you (@ChrisRackauckas) as I guess it could be a bit involved to weed out all cases of this and I want to make sure we get it right - is that OK?

Sure...ish? It's not something I can do right now, and I wouldn't put it high on my priorities, but hopefully I get around to it. But if I have some time and this is still open it's something I'll tackle.

Thanks!

@timholy Can I ask you, to which extent should I consider it a formal "best practice" to implement everything according to the generalized array styles? There are areas where it would be difficult (e.g. https://github.com/JuliaPlots/Plots.jl/blob/master/src/backends/gr.jl#L139-L177 ),
and length(linearindices(x)) feels a little heavy.

On the other hand, the fix suggested in my first post

import OffsetArrays
@recipe f{T<:OffsetArrays.OffsetArray}(::Type{T}, val::T) = val.parent

is a two-line fix that replaces all offsetarrays with normal rates at the beginning of each call - bug-free and hassle-free. After wading through all 285 instances of length(x) in Plots this feels tempting, but of course we'd like to align with formal best practice.

Is it only me or did this slip through to Julia 1.0 and further? 馃檲 This sadly makes unconventional indexing not feel like a first class feature of Julia.

julia> plot(OffsetArray([-10:10...], -11))
Error showing value of type Plots.Plot{Plots.GRBackend}:
ERROR: BoundsError: attempt to access 21-element OffsetArray(::Array{Float64,1}, -10:10) with eltype Float64 with indices -10:10 at index [1:21]

Yes, the PR never got merged, and is now hopelessly behind. It needs someone to reimplement it I'm afraid. But I agree it should be a priority.

I just submitted a PR (#2304) that should fix most issues.

Fixed by #2304

Was this page helpful?
0 / 5 - 0 ratings

Related issues

daschw picture daschw  路  3Comments

PallHaraldsson picture PallHaraldsson  路  4Comments

asinghvi17 picture asinghvi17  路  3Comments

Krastanov picture Krastanov  路  3Comments

pkofod picture pkofod  路  3Comments