search

sjmgarnier / viridis

Colorblind-Friendly Color Maps for R
http://sjmgarnier.github.io/viridis
Other
294 stars 38 forks source link

Core viridis as a separate package? #21

Closed zkamvar closed 8 years ago

zkamvar commented 8 years ago

Thank you for making viridis a reality in R. I hope never again to see a red/green heatmap in my life. I have a small request that might seem trivial: create a separate package containing the core elements for viridis (i.e. what you had in viridis version 0.1).

The reason I ask is for package maintainers who want to utilize the viridis color palette, but do not want to have ggplot2 or gridExtra as dependencies.

I know it is kind of a pain, especially after you've just submitted to CRAN, but I do know of at least one project that is planning to return to a red/yellow/green default because of the dependencies.

Please let me know if I'm completely daft :smile:

sjmgarnier commented 8 years ago

@hrbrmstr @noamross What's your opinion on that? It shouldn't be too difficult. We'd just have to put the core palette functions in a different package (maybe called viridisPalettes or something like this) and make the current viridis package depend on it.

hrbrmstr commented 8 years ago

I will refrain from commenting on the fear of relying on the size and longevity of two of the most used graphics packages and posit that there is some merit to this, as it also aligns with some of the current "micropackage" ideas floating around the #rstats ecosystem. CRAN won't be too upset by this either and there are no current reverse dependencies on viridis, so it would only require a small bit of extra communication regarding the change (and the timing is still good).

If you'd (@sjmgarnier) need any help with this (asking solely due to any time commitments you have) just lemme know.

sjmgarnier commented 8 years ago

@hrbrmstr Thanks for your feedback. I might have the time this weekend to create a separate package with minimal external dependencies. We could just release it separately for the moment, and then make viridis depends on it later if necessary (I don't see the core functions changing dramatically anytime soon, so it shouldn't be too much extra work).

What do you think? Since viridis is my first ever package on CRAN (and still the only one until I get a few things sorted out with editR), I'll rely on your experience for choosing the best course of action.

Also, we'll need a name. viridisPalettes seems appropriate, but I'm afraid it might cause confusion with the original viridis package. viridisLite maybe?

hrbrmstr commented 8 years ago

"There are only two hard things in Computer Science: cache invalidation and naming things."

I think, perhaps, we could just move the ggplot2 scales into a viridisScales package, split out the vignette into two (that's not too much extra work) so there is no reliance for it on the base viridis package in any way and keep core viridis as just the palettes? (that's a q not a "we should"). You wouldn't be stuck dealing with any ggplot2-related changes then (i.e. I will have to tweak the scale_… functions for the new ggplot2 coming out soon, at least I think I will).

I actually secretly (well, not anymore ;-) hope that the R Core team pings you to add viridis to grDevices which would mean having to do the ggplot-ish extraction anyway.

I could definitely take a stab at that this weekend if we end up going this route.

zkamvar commented 8 years ago

A small comment from the peanut gallery:

@hrbrmstr, if the scales are moved to a new package, wouldn't that require a major version increment for viridis as it would create breaking changes?

hrbrmstr commented 8 years ago

Yep. It would indeed. I'm still not sure why someone would not want to deal with the pkg due to dependencies on projects that will (IMO) no doubt outlive their package.

On Thu, Oct 15, 2015 at 4:07 PM, Zhian N. Kamvar notifications@github.com wrote:

A small comment from the peanut gallery:

@hrbrmstr https://github.com/hrbrmstr, if the scales are moved to a new package, wouldn't that require a major version increment for viridis as it would create breaking changes?

— Reply to this email directly or view it on GitHub https://github.com/sjmgarnier/viridis/issues/21#issuecomment-148506507.

sjmgarnier commented 8 years ago

@hrbrmstr I'm not sure what the reasoning is as well. Maybe limited computing resources or something like this.

Anyway, since it's likely to be for a small audience and a very specific usage, I think it might be less painful for us to just have a "lite" version of the package. No need for us to split the existing viridis package, we just have a separate package with the core functions for those who really don't want to load the extra dependencies. Since the core functions are unlikely to change much, it won't be hard for me to create and maintain the extra package.

zkamvar commented 8 years ago

Honestly, I don't really understand the reasoning myself. :confused: I'm just a messenger in this context. I agree that creating a viridisLite package and keeping the feature-rich viridis would be the best approach here.

sjmgarnier commented 8 years ago

Ok, then I'll try to get on this over the weekend. Not sure when it will be up on CRAN, but it should be accessible on Github next week.

sjmgarnier commented 8 years ago

@zkamvar Here you go: https://github.com/sjmgarnier/viridisLite.

ggplot2 is still listed in the suggested packages (but not the imported ones anymore) because I was just too busy (or lazy maybe) to modify the examples in the documentation. But this should not be a problem as it won't import anything when the package is loading.

Let me know if this work for you. The package 'checked' so it should not be too hard to get it into CRAN sometimes next week if it all looks good for you.

sjmgarnier commented 8 years ago

Closed by this: https://github.com/sjmgarnier/viridisLite

sjmgarnier commented 8 years ago

@zkamvar Any feedback on the lite version of viridis? Is it what you were looking for? If yes, I'll submit it to CRAN sometimes this week.

zkamvar commented 8 years ago

@sjmgarnier Everything works perfectly. My silent compatriot was delighted to find that the lite version steers clear of his dependency fears.

Please let us know when it's on CRAN. Thank you for all the hard work!

PS, He sent an example of what his package uses viridisLite for in case you wanted to see. hmbp_viridis

sjmgarnier commented 8 years ago

@zkamvar Perfect. I'll try to submit the package in the next few days.

sjmgarnier commented 8 years ago

@zkamvar Package submitted to CRAN. Sorry it took so long, things have been a bit chaotic over the last 3-4 weeks. I'll let you know as soon as the package is up on CRAN.

zkamvar commented 8 years ago

No worries! Chaos is what makes life interesting. Thanks for keeping me in the loop :)

On Thu, Nov 12, 2015 at 6:40 PM, Simon Garnier notifications@github.com wrote:

@zkamvar https://github.com/zkamvar Package submitted to CRAN. Sorry it took so long, things have been a bit chaotic over the last 3-4 weeks. I'll let you know as soon as the package is up on CRAN.

— Reply to this email directly or view it on GitHub https://github.com/sjmgarnier/viridis/issues/21#issuecomment-156304380.

sjmgarnier commented 8 years ago

@zkamvar It's on CRAN now: https://cran.r-project.org/web/packages/viridisLite/