search

JuliaGraphics / ColorSchemes.jl

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

Add `firstindex` method and tests #100

Closed stelmo closed 2 years ago

stelmo commented 2 years ago

Fixes #99 when merged. Adds firstindex method and adds some indexing tests.

stelmo commented 2 years ago

Not sure why 1.3 failed :/

cormullion commented 2 years ago

Not sure why 1.3 failed :/

firstindex() (or perhaps []begin) was introduced in Julia version v1.5 I think... 😀

rafaqz commented 2 years ago

Yeah begin didn't exist in 1.3. Might need a conditional to test it.

stelmo commented 2 years ago

Hmm, I added a conditional in the test (https://github.com/JuliaGraphics/ColorSchemes.jl/pull/100/files#r969470216) but it still doesn't pass 😕

stelmo commented 2 years ago

Hmm it seems to be a load error actually ERROR: LoadError: syntax: unexpected "]". Can I make a PR to drop support for testing against Julia 1.3? Or is there a special reason for using that version? Perhaps only test against v1.6, the current LTS, and nightly?

cormullion commented 2 years ago

I wonder if it could be done with eval?

if VERSION > v"1.5.0"
    eval((:Base.firstindex() = ...
end

Yes, specifying Julia as v1.6.0 (LTS) is probably a good idea, although there are a lot of packages that depend on ColorSchemes, so it should be considered in context.

cormullion commented 2 years ago

That's a pity... :(

stelmo commented 2 years ago

Yep, it didn't work :( Hmm I can remove the tests from the PR or I can make a PR that bumps up the testing requirements. However, it seems that the package might cause problems if anyone is still using Julia 1.3... Either way is fine with me though!

cormullion commented 2 years ago

To be honest, I'm tempted to just not test the begin syntax until we drop 1.3 support later, when we can add this test back in again... :) Prefer to let depending packages (Makie, Plots, etc) make the move first... :)

stelmo commented 2 years ago

No problem! I will put a to do note in the test and comment it out : )