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

alpha should be smart enough to handle values in [0, 1] automatically #37

Closed
MichaelChirico opened this issue Dec 4, 2018 · 6 comments
Closed
Assignees
Milestone

Comments

@MichaelChirico
Copy link
Contributor

@MichaelChirico MichaelChirico commented Dec 4, 2018

I think it's more common for people to think of alpha as a percentage, so I think it's natural to expect colourvalues(x, alpha = .5) to "just work".

From a design perspective, it's basically trivial to just detect fractions in alpha and multiply them by 255... simple truncation should be OK as I think the visual difference between alpha = 128 and alpha = 129 must be close to 0.

Things get interesting at the edge cases like alpha = c(0, 1, 1, 0)... I guess very few people will be using alpha = c(0, 1, 1, 0) on the 255 scale, so the logic to handle this requires some assumptions but I think they are pretty lightweight.

I'm happy to implement this functionality as a PR if you see fit.

@SymbolixAU
Copy link
Owner

@SymbolixAU SymbolixAU commented Dec 4, 2018

I think it's a simple implementation (on branch alpha), and is only necessary where alpha is a single value, since

colour_values(1:5, alpha = seq(0, 0.8, length.out = 5))

'just works' too.

However, what should colour_values(x, 1.0001) return? Is this example a too nuanced edge-case?

@SymbolixAU
Copy link
Owner

@SymbolixAU SymbolixAU commented Jan 1, 2019

@MichaelChirico If I were to implement this, should I change the accepted values of alpha from [0,255] to [1,256], so that it's more obvious that anything in [0,1] will be treated as a percentage?

@MichaelChirico
Copy link
Contributor Author

@MichaelChirico MichaelChirico commented Jan 1, 2019

@SymbolixAU I think no... only 1 is ambiguous since 0 is the same for both interpretations. And I think 0 should be 0, not 1...

Will come back to this again when I'm off vacation...

@SymbolixAU
Copy link
Owner

@SymbolixAU SymbolixAU commented Jan 12, 2019

I've merged this into master in this commit, so now alpha is

optional. Single value in [0,255] applied to all colours, or a decimal in [0, 1) (to indicate a percentage, noting 1 is excluded), or a vector of numeric values the same length as x. The numeric vector will be scaled into the range [0,255]. If a matrix palette is supplied this argument is ignored.

I'm still not entirely 100% sure about this... and I don't know why...

@SymbolixAU SymbolixAU self-assigned this Jan 12, 2019
@MichaelChirico
Copy link
Contributor Author

@MichaelChirico MichaelChirico commented Jan 13, 2019

Hopefully this is a case of the beauty of dynamically-typed languages... 😬

@SymbolixAU SymbolixAU added this to the v0.2.2 milestone Jan 13, 2019
@MichaelChirico
Copy link
Contributor Author

@MichaelChirico MichaelChirico commented Jan 14, 2019

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

Successfully merging a pull request may close this issue.

None yet
2 participants
You can’t perform that action at this time.