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

Consider exporting repel_boxes #88

Closed
zachary-foster opened this issue Nov 14, 2017 · 5 comments
Closed

Consider exporting repel_boxes #88

zachary-foster opened this issue Nov 14, 2017 · 5 comments

Comments

@zachary-foster
Copy link

Summary

Hi! Thanks for the great package!

I am developing a package that produces graphs using ggfittext and I wanted to add label overlap avoidance. Some labels should be repel, some others not, and the labels are all different sizes. I have been able to use your package successfully by calling the internal function repel_boxes, but this causes Notes on my R CMD Check that I would like to get rid of.

https://github.com/grunwaldlab/metacoder/blob/2bb32d0a5a3a0b7e843dc19a3ebbf7c744367469/R/heat_tree.R#L867-L874

Unexported objects imported by ':::' calls:
‘ggrepel:::repel_boxes’ ‘taxa:::convert_base’ ‘taxa:::data_used’
See the note in ?::: about the use of this operator.

Suggestions

Would you consider exporting repel_boxes? I think it is generally useful for applying your algorithm outside of ggplot2 and others might find it useful.

If you like, I can submit a pull request with the changes.

@slowkow
Copy link
Owner

slowkow commented Nov 14, 2017

Cool! I think exporting the function might be a reasonable path forward.

I guess the major benefit of not exporting is that I gain the freedom to completely rewrite it and hack on it without breaking dependencies.

Alternatively, you might actually prefer to copy my code into your package. That gives you more freedom to make the function do exactly what you want. This also protects you from future changes that might happen in my function. I think that gives you a valuable benefit of additional freedom to change the code at the cost of duplicating code across two packages.

Would you consider copying the code? What are your thoughts on the best approach here?

@zachary-foster
Copy link
Author

Thanks for the fast response @slowkow! I can certainly copy the code if you would prefer that to exporting. It is always nice to be able to rewrite things. Thanks for letting me use the code! If I do copy it, I will let people know in the documentation that it came from ggrepel.

Either way, a wrapper for repel_boxes that is designed to accept and return data without graphing in ggplot, might make ggrepl useful for a wider range of applications, including use in R base graphics.

@slowkow
Copy link
Owner

slowkow commented Nov 14, 2017

Ok, sounds like it might be best to copy the code. Please copy and modify as you wish with some kind of attribution. It's not the most elegant code in the entire world, but it works. Good luck!

I agree that repel_boxes might be useful outside ggrepel, but there are two caveats:

  1. The size of the plotting device needs to be known prior to calling repel_boxes. This means that some code is automatically re-run each time the plotting device is resized. The repel_boxes function (and lots of other stuff) gets called just after the resize.

  2. If my memory is right, the size of text on the plotting device is available only after creating a grob to hold it.

These two caveats make me think that repel_boxes is best suited to be embedded in the code that is invoked during the drawing loops, and it doesn't stand very well on its own. (I might be wrong about these caveats... double-check if this is important to your case.)

As a separate pull request, I would be very happy to include some code to repel text labels in base R graphics. I think it might be easy to implement by coping the graphics::text function and modifying it a bit. A few people have mentioned base R, but I don't use base R graphics very much and I don't have the time to work on this feature right now.

@zachary-foster
Copy link
Author

Sounds good, thanks!

Thanks for the info! As far as I can tell, those caveats don't apply to me because I calculate the coordinates of the bounding box for each text label and use ggfittext to fill the box, but I can see how they might be an issue to others.

Unfortunately, I don't have much base R plotting experience either, but I will keep it in mind.

Thanks again!

@slowkow slowkow closed this as completed Nov 14, 2017
@zachary-foster
Copy link
Author

I wanted to thank you again for letting me use your code! It really improved the visualizations in my package. The updated version with your code is on CRAN now. I just made this figure and was thinking how much better it is because the labels don't overlap.

potted_vs_field

So much better than before! Thanks again!

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

No branches or pull requests

2 participants