-
Notifications
You must be signed in to change notification settings - Fork 1
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
Allow for discrete versions of gradients #131
Conversation
This reverts commit b07fc89.
Reorder and add note about discrete versions of gradients.
Note - this passed check on my machine. It seems there is an issue with the pkgdown deployment to the test branch, I would suggest we re-run later in case it's a temporary glitch (unless @nmpeterson or @matthewstern have encountered this before / know what it might be a result of?). Here's the error:
|
FWIW, rerunning the previously-successful pkgdown action on the most recent commit to the version1.2 branch (ebdbb66) creates a similar error situation and fails. Comparing the GHA log for a successful vs unsuccessful run shows entirely different response to the "install dependencies" code. I am hoping these failures have something to with a package update elsewhere, and this will resolve itself automatically in a few days. |
Mainly doing this to start a new check
@dlcomeaux FYI, you can re-run a failed check without a new commit. Just go to the Actions tab, open the failed action you want to re-run, then click "Re-run jobs" |
Also, regarding the current error with the textshaping package, just keep an eye on its CRAN page. Once the macOS release binary version (currently 0.3.3) matches the version listed at the top (0.3.4), the action should work. The way the pkgdown action is configured, it tries to install the latest version of every dependency, but if the binaries haven't been updated then they can't be installed. Interestingly, the same issue does not occur for the R-CMD-check action. |
Just for internal references
@nmpeterson the binaries have been updated, and everything seems to be in order again. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am OK with this changes but would like @gritzenthaler to confirm before merging.
I've been rather inactive on this project lately but getting to this now! I'll review it by the end of the day. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks great, thanks Daniel!
This is a relatively simple PR - it just adds the gradients to the list of allowed color palettes for the discrete scales. I also updated the documentation and vignettes accordingly. I could be missing something, so please let me know @gritzenthaler if there's some reason why this wouldn't work in practice.
One thing I was thinking about as additional work might be to find a way to "recenter" a diverging discrete palette around some non-mid-point using factors (I can see instances where this might come up in a mapping context, i.e., you have five bins above zero and two below - the current code would center it around the 4th factor, no matter the value). However, I couldn't see an easy way to do that, since the discrete scales don't seem to have a rescaling option easily available.
Open to other thoughts - I thought at the very least this adds the requested functionality and doesn't preclude a more sophisticated approach later on.