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

encode_colour accepts a list besides a matrix #36

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

zeehio
Copy link

@zeehio zeehio commented Sep 18, 2022

This pull request lets encode_colour() accept a list or a data frame instead of a colour matrix.

This has performance improvements when each channel is computed independently, because instead of cbind()-ing them together we can list() them, avoiding the allocation of a larger matrix.

This is a first subset of #35, aiming to improve performance in farver with larger vectors. It is part of tidyverse/ggplot2#4989, to improve the performance of rasterizing large matrices.

No breaking changes are introduced.

Each commit can be reviewed independently.

Thanks for your time

@zeehio
Copy link
Author

zeehio commented May 10, 2023

Hi @thomasp85

No hurries. It is not my intention to put any pressure on you.

As you know, I contributed several pull requests for farver. In case they are of interest to you and you plan to eventually review them I would appreciate to know in advance when you are planning to do it (e.g. on the first week of September I'd appreciate if you would tell me "in two weeks I'll look into some of them").

I understand how hard it is to find time for these things and possible I would like to have some time available when you do, so I can reply as fast as possible during your time slot.

It does not need to be this month or even this quarter. I don't want to push or mess with anyone's agenda, I just would like to do my best to align my time to your availability.

Thanks for all your work!

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 this pull request may close these issues.

1 participant