search

JuliaGraphics / ColorSchemes.jl

colorschemes, colormaps, gradients, and palettes
http://juliagraphics.github.io/ColorSchemes.jl/
Other
190 stars 35 forks source link

Use tuples instead of StaticArrays #42

Closed rafaqz closed 3 years ago

rafaqz commented 4 years ago

As @KristofferC just mentioned in #33 we could use a tuple instead of a Vector for colors chemes, and remove the StaticArrays.jl dependency.

Is there any reason colors need to be in a Vector ? I (probably wrongly) assumed there was when I added the dependency to StaticArrays.jl to speed up color-scheme indexing. Tuples are cleaner and would improve package load time a lot, and the load time of other packages like Gaston.jl.

I'll write the PR if there is no problem with making the change.

cormullion commented 4 years ago

Sounds good to me - it's a long time since I made this package (Julia v0.4) ... :) Make sure you don't break the entire plotting ecosystem... :)

rafaqz commented 4 years ago

Eeeek that was my worry... We would have to test all the dependent packages before merging.

mbaz commented 4 years ago

Maybe this change merits a major, breaking release? That way you wouldn't (technically) break any package that depends on ColorSchemes. I can't speak for everybody but I'd welcome the improved load times.

KristofferC commented 4 years ago

Why would this be a breaking change? Are people doing math with the color vector? It's just a list of colors.

rafaqz commented 4 years ago

This mostly works and package load time is less than half of what it is with StaticArrays, but documentation breaks https://travis-ci.com/github/rafaqz/ColorSchemes.jl/jobs/402345022

I think using Tuples loses a bunch of show methods for vectors of colors. So they will also no longer show in atom etc. I'm not sure how much work that is to fix.

cormullion commented 4 years ago

I don't think there are any tests for show at the moment... :( I could fix the documentation to not automatically insert a colorscheme by default, but it would be better if there was some kind of show method perhaps.

rafaqz commented 4 years ago

I really like seeing the colorscheme in atom - I guess it would mean adding a method here in Colors.jl? https://github.com/JuliaGraphics/Colors.jl/blob/0890b410940fa9645496a394e5c449ef7104769a/src/display.jl#L26

The AbstractArray interface does seem kinda handy ;) I'm wondering if it's worth losing the simplicity of that for 1 second of start time...

rafaqz commented 4 years ago

Maybe ColorScheme could itself be in <: AbstractVector and that would be solved. It already defines getindex the whole abstract array interface, it may as well be an AbstractVector