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

WIP: basic outline of base version of repel #157

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

MichaelChirico
Copy link
Contributor

@MichaelChirico MichaelChirico commented May 7, 2020

Some initial work on #156

The skeleton is there -- API I envision is expose repel_text (or maybe text_repel) as the "repel" version of graphics::text.default

TODO:

  • Implement pos argument
  • Implement offset argument
  • Thorough testing of expected outputs
  • Don't draw segments if the text "hasn't moved" (?)
  • Examples
  • NEWS
  • Clean-up

@MichaelChirico
Copy link
Contributor Author

@slowkow any general comments on API / expected behavior / style are welcome at any time

@slowkow
Copy link
Owner

slowkow commented May 7, 2020

Thanks for working on this! I think I might prefer text_repel() instead of repel_text().

I don't use base graphics very often, so I'm not sure what the preferred interface would be for someone who does. We might want to ask for comments from someone who does use base when you're ready.

When you're ready, could I please ask if you might be able to share a figure that shows this working?

Does the figure redraw if you resize the window? Like this.

@MichaelChirico
Copy link
Contributor Author

Does the figure redraw if you resize the window?

I can't imagine it would 🤔

I guess ggplot2 is handling that through its special infra -- resizing the plot window re-induces print.ggplot2, which can re-calculate the plot elements.

This could be captured by a dynamic plotting environment for base (e.g. on shiny), but I am not sure it's possible for unadorned base plots.

@MichaelChirico
Copy link
Contributor Author

@slowkow I added test-base-plots.R (I'll delete it or migrate it before merge), plan is to do a ton of test plots in there.

@MichaelChirico
Copy link
Contributor Author

@slowkow now tracking plots in this PR under test-base/figures

@slowkow
Copy link
Owner

slowkow commented Jul 23, 2020

Thanks for working on this! Let me know when you want me to have a deeper look. I can be slow to respond and might need multiple reminders.

@MichaelChirico
Copy link
Contributor Author

Thanks @slowkow -- IIRC there was an issue where the results are highly volatile. Maybe that's an issue you've seen with the gg version of the algorithm as well. I haven't had time to get back to investigate more...

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.

2 participants