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_values() modifying global environment? #60

Closed
ebuhle opened this issue Jan 22, 2020 · 3 comments
Closed

color_values() modifying global environment? #60

ebuhle opened this issue Jan 22, 2020 · 3 comments
Labels
bug Something isn't working

Comments

@ebuhle
Copy link

ebuhle commented Jan 22, 2020

Issue description

Satisfied colourvalues user here, but encountering some very strange behavior after updating R (session info below). When color_values() is called, it modifies not only the object passed as the x argument, but also other object(s) that are copies of x.

This seems related to #24, but that issue focused on the difference between integer and numeric arguments. My question is rather how the function could be modifying the object in the global environment that is passed to it. This is very unexpected behavior in R, and even more surprising is that copies of that object are modified. This breaks everything I thought I knew about R.

Session info

In addition to the following info for my system, a colleague has reproduced this on a Mac (don't know session info, but would assume everything up to date).

R version 3.6.2 (2019-12-12)
Platform: x86_64-w64-mingw32/x64 (64-bit)
Running under: Windows 7 x64 (build 7601) Service Pack 1

Matrix products: default

locale:
  [1] LC_COLLATE=English_United States.1252  LC_CTYPE=English_United States.1252   
[3] LC_MONETARY=English_United States.1252 LC_NUMERIC=C                          
[5] LC_TIME=English_United States.1252    

attached base packages:
  [1] stats     graphics  grDevices utils     datasets  methods   base     

other attached packages:
  [1] viridis_0.5.1      viridisLite_0.3.0  colourvalues_0.3.1

loaded via a namespace (and not attached):
  [1] splines_3.6.2         gtools_3.8.1          StanHeaders_2.19.0   
[4] threejs_0.3.1         shiny_1.4.0           assertthat_0.2.1     
[7] stats4_3.6.2          pillar_1.4.3          lattice_0.20-38      
[10] glue_1.3.1            digest_0.6.23         promises_1.1.0       
[13] minqa_1.2.4           colorspace_1.4-1      htmltools_0.4.0      
[16] httpuv_1.5.2          Matrix_1.2-18         plyr_1.8.5           
[19] dygraphs_1.1.1.6      pkgconfig_2.0.3       rstan_2.19.2         
[22] purrr_0.3.3           xtable_1.8-4          scales_1.1.0         
[25] processx_3.4.1        later_1.0.0           lme4_1.1-21          
[28] tibble_2.1.3          bayesplot_1.7.1       ggplot2_3.2.1        
[31] DT_0.11               shinyjs_1.0           lazyeval_0.2.2       
[34] cli_2.0.0             survival_3.1-8        magrittr_1.5         
[37] crayon_1.3.4          mime_0.8              ps_1.3.0             
[40] fansi_0.4.0           nlme_3.1-143          MASS_7.3-51.5        
[43] xts_0.11-2            pkgbuild_1.0.6        colourpicker_1.0     
[46] rsconnect_0.8.16      tools_3.6.2           loo_2.2.0            
[49] prettyunits_1.0.2     lifecycle_0.1.0       matrixStats_0.55.0   
[52] stringr_1.4.0         munsell_0.5.0         packrat_0.5.0        
[55] callr_3.4.0           compiler_3.6.2        rlang_0.4.2          
[58] grid_3.6.2            nloptr_1.2.1          ggridges_0.5.1       
[61] rstudioapi_0.10       rstanarm_2.19.2       htmlwidgets_1.5.1    
[64] crosstalk_1.0.0       igraph_1.2.4.2        miniUI_0.1.1.1       
[67] base64enc_0.1-3       boot_1.3-24           codetools_0.2-16     
[70] gtable_0.3.0          inline_0.3.15         markdown_1.1         
[73] reshape2_1.4.3        R6_2.4.1              gridExtra_2.3        
[76] rstantools_2.0.0.9000 zoo_1.8-6             dplyr_0.8.3          
[79] fastmap_1.0.1         shinystan_2.5.0       shinythemes_1.1.2    
[82] stringi_1.4.3         parallel_3.6.2        Rcpp_1.0.3           
[85] tidyselect_0.2.5     

Reprex

library(colourvalues)
library(viridis)
set.seed(123)

x <- runif(10,-10,10)
y <- x
x
y
z <- color_values(y, palette = t(col2rgb(viridis::cividis(256, direction = -1))))
x
y

Expected result

The objects y and, certainly, x are not affected by calling color_values().

Actual result

> x <- runif(10,-10,10)
> y <- x
> x
 [1] -4.2484496  5.7661027 -1.8204616  7.6603481  8.8093457 -9.0888700  0.5621098
 [8]  7.8483809  1.0287003 -0.8677053
> y
 [1] -4.2484496  5.7661027 -1.8204616  7.6603481  8.8093457 -9.0888700  0.5621098
 [8]  7.8483809  1.0287003 -0.8677053
> z <- color_values(y, palette = t(col2rgb(cividis(256, direction = -1))))
> x
 [1] 0.2704415 0.8299695 0.4060968 0.9358038 1.0000000 0.0000000 0.5392146 0.9463095
 [9] 0.5652837 0.4593287
> y
 [1] 0.2704415 0.8299695 0.4060968 0.9358038 1.0000000 0.0000000 0.5392146 0.9463095
 [9] 0.5652837 0.4593287

I've been using colourvalues for a year or so and I've never seen any strange behavior like this until now. In scripts, code that worked fine before now has this unexpected side effect. (I noticed it after updating from R 3.6.1 to 3.6.2 and updating packages, though I haven't tried rolling anything back.)

@SymbolixAU SymbolixAU added the bug Something isn't working label Jan 22, 2020
SymbolixAU pushed a commit that referenced this issue Jan 22, 2020
@SymbolixAU
Copy link
Collaborator

I think this was directly related to #24 - I missed a clone statement.

Please try the latest version from github

devtools::install_github("SymbolixAU/colourvalues")

Then you should see the expected behaviour

library(colourvalues)

set.seed(123)

x <- runif(10,-10,10)
y <- x
x
y
z <- color_values(y, palette = get_palette("cividis")[256:1, ] )
x
y
z

Note also that colourvalues has a get_palette() function for getting various palettes.

@ebuhle
Copy link
Author

ebuhle commented Jan 22, 2020

Thanks very much, @SymbolixAU. The GitHub version does indeed resolve the issue. I gather the clone statement is on the C++ side, so I'll shield my eyes and pretend I never saw this. :-)

And thanks for the tip about get_palette(); I knew there were various options but I wasn't clever enough to figure out how to reverse the direction.

@SymbolixAU
Copy link
Collaborator

yeah it's a C++ thing :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

1 participant