Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Color parser is both strict and loose #357

Closed
kimikage opened this issue Sep 25, 2019 · 1 comment · Fixed by #360
Closed

Color parser is both strict and loose #357

kimikage opened this issue Sep 25, 2019 · 1 comment · Fixed by #360

Comments

@kimikage
Copy link
Collaborator

The following styles are generally valid according to the CSS Level 3, but are rejected in Colors.jl.

julia> parse(RGBA,"rgba(1,10,100,.5)")
ERROR: Unknown color: rgba(1,10,100,.5)

julia> parse(RGBA,"rgba(1,10,100,\t0.5)")
ERROR: Unknown color: rgba(1,10,100,       0.5)

Although we do not have to support parsing the all numerical notations, I think it is worth supporting the former because the former is shorter than something like +5e-1.

On the other hand, the following styles are accepted without errors or warnings.

julia> parse(Colorant,"#FF8000demo")
RGB{N0f8}(1.0,0.502,0.0)

julia> parse(RGB,"p ink")
RGB{N0f8}(1.0,0.753,0.796)

The former is potentially dangerous since Colors.jl does not support parsing the 8-digit or 4-digit hex notation yet, as mentioned in #353.
The latter is not so dangerous, but the replace() is slightly slow.

desc_ = replace(desc, " " => "")

In the cases where the regex matching is used, there is no need to replace the input string. In the case of the named colors, strip(), which returns SubString, may be helpful.

@kimikage
Copy link
Collaborator Author

kimikage commented Sep 29, 2019

I will send the PR after merging #356.
kimikage@2dcec49

Benchmark

Since the latency characteristics of string operations are quite different between Windows (MinGW) and Linux, it is hard to satisfy both sides.
I have no modern macOS environment and I don't have any natively-installed x86_64 Linux systems.

Linux (x86_64-linux-gnu) - Debian 10 on WSL

Julia v1.0.3

target before(43d93ff) after
"#C0FFEE" 444.949 ns (15 allocations: 752 bytes) 237.176 ns (7 allocations: 352 bytes)
"rgb(0,0,0)" 528.272 ns (14 allocations: 720 bytes) 380.882 ns (7 allocations: 400 bytes)
"rgba( 0, 11,222,0.45)" 909.091 ns (15 allocations: 768 bytes) 665.000 ns (8 allocations: 432 bytes)
"LightGoldenRodYellow" 565.405 ns (12 allocations: 608 bytes) 505.699 ns (11 allocations: 400 bytes)
"lightgoldenrodyellow" 561.290 ns (12 allocations: 608 bytes) 423.618 ns (8 allocations: 224 bytes)

Julia v1.2.0

target before(43d93ff) after
"#C0FFEE" 382.178 ns (15 allocations: 736 bytes) 190.723 ns (7 allocations: 352 bytes)
"rgb(0,0,0)" 472.449 ns (14 allocations: 704 bytes) 336.199 ns (8 allocations: 416 bytes)
"rgba( 0, 11,222,0.45)" 816.854 ns (15 allocations: 752 bytes) 604.520 ns (9 allocations: 448 bytes)
"LightGoldenRodYellow" 506.736 ns (11 allocations: 560 bytes) 445.455 ns (7 allocations: 320 bytes)
"lightgoldenrodyellow" 496.907 ns (11 allocations: 560 bytes) 322.609 ns (5 allocations: 176 bytes)

Windows (x86_64-w64-mingw32) - Windows 10

Julia v1.2.0

target before(43d93ff) after
"#C0FFEE" 462.431 ns (15 allocations: 736 bytes) 226.293 ns (7 allocations: 352 bytes)
"rgb(0,0,0)" 560.215 ns (14 allocations: 704 bytes) 397.512 ns (8 allocations: 416 bytes)
"rgba( 0, 11,222,0.45)" 1.390 μs (15 allocations: 752 bytes) 1.110 μs (9 allocations: 448 bytes)
"LightGoldenRodYellow" 622.353 ns (11 allocations: 560 bytes) 548.663 ns (7 allocations: 320 bytes)
"lightgoldenrodyellow" 603.955 ns (11 allocations: 560 bytes) 369.417 ns (5 allocations: 176 bytes)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant