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

Bad repelled-location of labels #92

Closed
aphalo opened this issue Dec 18, 2017 · 7 comments
Closed

Bad repelled-location of labels #92

aphalo opened this issue Dec 18, 2017 · 7 comments

Comments

@aphalo
Copy link
Contributor

aphalo commented Dec 18, 2017

Summary

I find that in some cases locations of labels seem to be displaced, as if some indexing is off by one. Until now, I have seen this only happening with labels bellow a line, but not above. Not with text_repel, but maybe just a question of available space.

Minimal code example

I will add a minimum example later today, but as you may be getting ready to submit the new version, this is shown as a warning of a proper example I will add by the end of the day.

This example demonstrates the issue, although it is not a minimum one.

library(ggplot2)
library(photobiology)
library(ggspectra)
ggplot(sun.spct) + geom_line() + 
    stat_label_valleys(geom = "label_repel", span = 51, 
                                nudge_y = -0.075, segment.colour = "black", 
                                colour = "black", fill = "white")
ggplot(sun.spct) + geom_line() + 
    stat_label_valleys(geom = "text_repel", span = 51, 
                                nudge_y = -0.075, segment.colour = "black", 
                                colour = "black")

Here is an image of the output produced by the code:

image

image

Suggestions

I have no proposal at the moment. This seems to be a bug.

Version information

Here is the output from sessionInfo() in my R session:

Session info -----------------------------------------------------------------------------------
 setting  value                                      
 version  R version 3.4.3 Patched (2017-12-14 r73916)
 system   x86_64, mingw32                            
 ui       RStudio (1.1.399)                          
 language (EN)                                       
 collate  English_United Kingdom.1252                
 tz       Europe/Helsinki                            
 date     2017-12-18                                 

Packages ---------------------------------------------------------------------------------------
 package               * version    date       source                            
 base                  * 3.4.3      2017-12-15 local                             
 colorspace              1.3-2      2016-12-14 CRAN (R 3.4.0)                    
 compiler                3.4.3      2017-12-15 local                             
 curl                    3.1        2017-12-12 CRAN (R 3.4.3)                    
 datasets              * 3.4.3      2017-12-15 local                             
 devtools              * 1.13.4     2017-11-09 CRAN (R 3.4.2)                    
 digest                  0.6.13     2017-12-14 CRAN (R 3.4.3)                    
 ggplot2               * 2.2.1.9000 2017-12-15 Github (tidyverse/ggplot2@bfff1d8)
 ggrepel               * 0.7.1      2017-12-16 Github (slowkow/ggrepel@3f5d03f)  
 ggspectra             * 0.2.3      2017-12-16 local                             
 git2r                   0.19.0     2017-07-19 CRAN (R 3.4.1)                    
 graphics              * 3.4.3      2017-12-15 local                             
 grDevices             * 3.4.3      2017-12-15 local                             
 grid                    3.4.3      2017-12-15 local                             
 gtable                  0.2.0      2016-02-26 CRAN (R 3.4.0)                    
 httr                    1.3.1      2017-08-20 CRAN (R 3.4.1)                    
 labeling                0.3        2014-08-23 CRAN (R 3.4.0)                    
 lazyeval                0.2.1      2017-10-29 CRAN (R 3.4.2)                    
 memoise                 1.1.0      2017-04-21 CRAN (R 3.4.0)                    
 methods               * 3.4.3      2017-12-15 local                             
 munsell                 0.4.3      2016-02-13 CRAN (R 3.4.0)                    
 photobiology          * 0.9.18-1   2017-12-16 local                             
 photobiologyWavebands   0.4.2      2017-03-20 CRAN (R 3.4.0)                    
 plyr                    1.8.4      2016-06-08 CRAN (R 3.4.0)                    
 R6                      2.2.2      2017-06-17 CRAN (R 3.4.0)                    
 Rcpp                    0.12.14    2017-11-23 CRAN (R 3.4.2)                    
 rlang                   0.1.4.9000 2017-12-16 Github (tidyverse/rlang@cc7587c)  
 rstudioapi              0.7        2017-09-07 CRAN (R 3.4.1)                    
 scales                  0.5.0.9000 2017-09-01 Github (hadley/scales@d767915)    
 splus2R                 1.2-2      2016-09-02 CRAN (R 3.4.0)                    
 stats                 * 3.4.3      2017-12-15 local                             
 tibble                  1.3.4      2017-08-22 CRAN (R 3.4.1)                    
 tools                   3.4.3      2017-12-15 local                             
 utils                 * 3.4.3      2017-12-15 local                             
 withr                   2.1.0.9000 2017-11-25 Github (jimhester/withr@fe81c00)  
 yaml                    2.1.16     2017-12-12 CRAN (R 3.4.3)       
@aphalo
Copy link
Contributor Author

aphalo commented Dec 18, 2017

Hello again,
The current CRAN version works fine. The code above produces:
image

Session info -----------------------------------------------------------------------------------
 setting  value                                      
 version  R version 3.4.3 Patched (2017-12-14 r73916)
 system   x86_64, mingw32                            
 ui       RStudio (1.1.399)                          
 language (EN)                                       
 collate  English_United Kingdom.1252                
 tz       Europe/Helsinki                            
 date     2017-12-18                                 

Packages ---------------------------------------------------------------------------------------
 package               * version    date       source                            
 assertthat              0.2.0      2017-04-11 CRAN (R 3.4.0)                    
 base                  * 3.4.3      2017-12-15 local                             
 bindr                   0.1        2016-11-13 CRAN (R 3.4.0)                    
 bindrcpp              * 0.2        2017-06-17 CRAN (R 3.4.0)                    
 colorspace              1.3-2      2016-12-14 CRAN (R 3.4.0)                    
 compiler                3.4.3      2017-12-15 local                             
 datasets              * 3.4.3      2017-12-15 local                             
 devtools              * 1.13.4     2017-11-09 CRAN (R 3.4.2)                    
 digest                  0.6.13     2017-12-14 CRAN (R 3.4.3)                    
 dplyr                 * 0.7.4      2017-09-28 CRAN (R 3.4.2)                    
 ggplot2               * 2.2.1.9000 2017-12-15 Github (tidyverse/ggplot2@bfff1d8)
 ggrepel                 0.7.0      2017-09-29 CRAN (R 3.4.3)                    
 ggspectra             * 0.2.3      2017-12-18 local                             
 glue                    1.2.0      2017-10-29 CRAN (R 3.4.2)                    
 graphics              * 3.4.3      2017-12-15 local                             
 grDevices             * 3.4.3      2017-12-15 local                             
 grid                    3.4.3      2017-12-15 local                             
 gtable                  0.2.0      2016-02-26 CRAN (R 3.4.0)                    
 labeling                0.3        2014-08-23 CRAN (R 3.4.0)                    
 lazyeval                0.2.1      2017-10-29 CRAN (R 3.4.2)                    
 magrittr                1.5        2014-11-22 CRAN (R 3.4.0)                    
 memoise                 1.1.0      2017-04-21 CRAN (R 3.4.0)                    
 methods               * 3.4.3      2017-12-15 local                             
 munsell                 0.4.3      2016-02-13 CRAN (R 3.4.0)                    
 photobiology          * 0.9.18-1   2017-12-16 local                             
 photobiologyWavebands * 0.4.2      2017-03-20 CRAN (R 3.4.0)                    
 pkgconfig               2.0.1      2017-03-21 CRAN (R 3.4.0)                    
 plyr                    1.8.4      2016-06-08 CRAN (R 3.4.0)                    
 R6                      2.2.2      2017-06-17 CRAN (R 3.4.0)                    
 Rcpp                    0.12.14    2017-11-23 CRAN (R 3.4.2)                    
 rlang                   0.1.4.9000 2017-12-16 Github (tidyverse/rlang@cc7587c)  
 rstudioapi              0.7        2017-09-07 CRAN (R 3.4.1)                    
 scales                  0.5.0.9000 2017-09-01 Github (hadley/scales@d767915)    
 splus2R                 1.2-2      2016-09-02 CRAN (R 3.4.0)                    
 stats                 * 3.4.3      2017-12-15 local                             
 tibble                  1.3.4      2017-08-22 CRAN (R 3.4.1)                    
 tools                   3.4.3      2017-12-15 local                             
 utils                 * 3.4.3      2017-12-15 local                             
 withr                   2.1.0.9000 2017-11-25 Github (jimhester/withr@fe81c00)  
 yaml                    2.1.16     2017-12-12 CRAN (R 3.4.3)                    
> 

@slowkow
Copy link
Owner

slowkow commented Dec 19, 2017

Always a pleasure to get an issue report from you, Pedro 😄

I agree that this is a bug, and it is probably caused by the new code that tries to resolve line segment overlaps.

Thanks for the minimal example. This will be crucial to test if new code fixes the bug.

If you have any ideas or want to give it a try yourself, please feel free! I'd be happy to add anyone as a contributor.

Unfortunately, I must delay fixing this issue for now, so I recommend that you continue using the CRAN version that does not support hjust and vjust.

Thanks again! You saved me from pushing a buggy version to CRAN 👍

@aphalo
Copy link
Contributor Author

aphalo commented Dec 19, 2017

No problem. I will be busy in the next few days so I won't be able to give it a try soon. I do not really need the new features, but I use ggrepel in the vignette of one of my own packages of which I have a new version ready to submit to CRAN. Because of this tested it with your upcoming release. If I find time to contribute to ggrepel, I think that as I am not familiar with your code, my most useful contribution would be to prepare some automated visual tests cases. I have added them to one of my packages and I am finding them very useful to detect glitches like this one. Sorry that my example is not that minimal as it uses spectral data that has more than a thousand observations!
Best wishes, and thanks for this extremely useful package! 😄

@slowkow
Copy link
Owner

slowkow commented Dec 22, 2017

I reproduced the bug with another very minimal example. This is not good... gotta find a quick way to fix this. I guess the CRAN version is best for now.

library(ggrepel)
set.seed(42)
# Make 100 points
dat <- data.frame(
  x = rnorm(100),
  y = rnorm(100),
  text = sample(letters, 100, replace = TRUE),
  stringsAsFactors = FALSE
)
# Label 10 points
dat_text <- dat[sample(nrow(dat), 10),]
# Let's avoid ALL data points.
dat_text2 <- dat
dat_text2$text[!rownames(dat_text2) %in% rownames(dat_text)] <- ""
ggplot() +
  geom_point(
    data = dat,
    mapping = aes(x, y)
  ) +
  geom_label_repel(
    data = dat_text2,
    mapping = aes(x, y, label = text)
  )
ggsave("issue92.png", width = 4, height = 4)

ggrepel 0.7.2

issue92-0.7.2

ggrepel 0.7.0 (CRAN)

issue92-0.7.0

@AliciaSchep
Copy link
Contributor

I put in a PR to fix this problem (that was introduced by the line cross checking code -- sorry about that! Hadn't tested the scenario with only labeling a subset of points with 'label' rather than 'text'). Thanks for catching this issue @aphalo and for the minimal reprex @slowkow.

I agree with @aphalo that some automated visual tests would be an awesome addition to this package! The vdiffr package might be a good approach

@aphalo
Copy link
Contributor Author

aphalo commented Dec 23, 2017

@AliciaSchep @slowkow I have started adding visual tests with vdiffr. I hope to have a PR in a week or so.

@slowkow
Copy link
Owner

slowkow commented Jan 29, 2018

I believe Alicia's pull request #94 fixes this.

@slowkow slowkow closed this as completed Jan 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants