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

hjust and vjust ( + line connection + line cross checking) #86

Merged
merged 7 commits into from
Nov 19, 2017

Conversation

AliciaSchep
Copy link
Contributor

This pull request includes adding support for hjust and vjust, as well as two additional changes that were important for having the plots using hjust and vjust actually look reasonable, following up discussion on #69.

The two major additional changes are:

  • Checking in repel_boxes every few iterations for whether lines are crossing, and attempting to swap label positions if that would help resolve the cross.
  • Different line connection method -- rather than connect to center, first find the closest point on rectangle. Then nudge connection point towards center depending on how far away the point is.

That second change impacts most plots with default args as well. I think it looks decent, but totally understand if you disagree!

Demonstration of hjust:

set.seed(42)

ggplot(mtcars, aes(y = wt, x = 1)) +
  geom_point(color = 'red') +
  geom_text_repel(aes(label = rownames(mtcars)), nudge_x = 0.2, 
                  direction = "y", 
                  hjust = 1) + 
  theme_classic(base_size = 16) + ylim(1,6) + 
  xlim(1,1.25) + 
  theme(axis.line.x = element_blank(),
        axis.ticks.x = element_blank(),
        axis.text.x = element_blank(),
        axis.title.x = element_blank())

image

@slowkow
Copy link
Owner

slowkow commented Nov 9, 2017

Thanks again for contributing! I really appreciate your work. 😄

I need a few days to get back to this, but so far your code works very well so I think I'll merge with very minimal modifications.

I noticed your figures are pretty big, so I wondered if I could squeeze them down a bit, and it seems that I can! Awesome! 👍


The code I'm running:

set.seed(42)

ggplot(mtcars, aes(x = wt, y = 1)) +
  geom_point(color = 'red') +
  geom_text_repel(
    aes(label = rownames(mtcars)),
    nudge_y = 0.05,
    direction = "x",
    angle = 90,
    vjust = 0,
    segment.size = 0.2
  ) + 
  theme_classic(base_size = 16) +
  xlim(1, 6) +
  ylim(1, 0.8) + 
  theme(
    axis.line.y = element_blank(),
    axis.ticks.y = element_blank(),
    axis.text.y = element_blank(),
    axis.title.y = element_blank()
  )

fig.width=9, fig.height=3 looks great!

image

@AliciaSchep
Copy link
Contributor Author

Great! Also - happy to follow up on any problems or questions related to this PR, just let me know 😄

@slowkow slowkow merged commit a59f6bd into slowkow:master Nov 19, 2017
@slowkow
Copy link
Owner

slowkow commented Dec 22, 2017

Hi @AliciaSchep, it seems that the line crossing algorithm is not working in some cases like issue #92.

If you have any ideas to fix this without losing the line crossing goodness, please feel free to make another pull.

I'll also try to fix this myself if I get a chance, but I might not get around to it for a while. For now I would recommend everyone to use the CRAN version 0.7.0 if they run into this bug.

Random thoughts:

I fear this might be a challenging one... sometimes I wonder if it would be best to throw away my hacky repulsion code and import some well-tested 2D physics engine like Box2D or similar. I worry ggrepel is reinventing the wheel a bit. I also feel like hacky repulsion code solves 80% of the problems, but 20% of problems still require hand-tuning and it's too much effort to try to solve them with code.

@AliciaSchep
Copy link
Contributor Author

Hi @slowkow, thanks for the heads up -- I will take a look into what is causing problems. Agree that examples in #92 appear very problematic!

@AliciaSchep
Copy link
Contributor Author

Update: I have figured out what the cause of the bug is...

The problem is that when only some points are labelled the correspondence between points and labels is messed up and labels end up getting associated with wrong point. The not-very-elegant solution (which I had implemented for geom_text_repel but forgot to also implement for geom_label_repel) is to re-order the points so that the labelled ones are first.

I will submit a PR later today with a fix for this bug #92.

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.

None yet

2 participants