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

Roadmap #50

Closed
teunbrand opened this issue Jan 9, 2022 · 56 comments
Closed

Roadmap #50

teunbrand opened this issue Jan 9, 2022 · 56 comments

Comments

@teunbrand
Copy link
Collaborator

Hi Alan,

We've come really far with this package and since we finished some major issues such as text smoothing, sf-text and richtext, I thought it is a good time to discuss what the next steps would be. It would be nice for example if we'd submit the package to CRAN at some point.

First, do still have some issues on your wish list that you'd be keen to see implemented in a first version? Personally, on the shorter term, I'd like to complete the vignettes, for example.

Next, do we want to announce the package before submitting to CRAN? I know that because we've been collaborating on this package publicly, some people have already picked up on this package and shared it on the social internet. Nonetheless, it might be nice to register the package in the ggplot2 extension gallery and/or announce on twitter that we think it is ready, and maybe solicit some feedback.

Lastly, before submitting to CRAN, we should probably do most of the items in the checklist generated by usethis:::release_checklist(). In particular, the rhub::check_for_cran() step has helped me previously.

@AllanCameron
Copy link
Owner

Hi Teun

I have been thinking much the same as yourself. I don't think there are any other major features we need to incorporate, and it would be good to get this on CRAN.

Although I would like us to submit it to the ggplot extension gallery, it would be better if the package was already on CRAN, since the extension gallery's default view seems to filter out non-CRAN packages.

There are a few things I would want to address before the CRAN stage:

  1. I'm not 100% happy with the text smoothing (or more specifically with the way it maps onto the 0 - 100 scale). I'll need to tweak this.
  2. Having got my head round the rich text mechanism, I have simplified the code and made it easier to include methods for other tags. Did you want to include any other rich text tags? I'm not sure that any of them are very useful for plot labels, other than underline, which would be more difficult to implement. Maybe <small> and <big>, which would be trivial to implement?
  3. Do we want to put the ribbon back into geom_textsmooth and geom_labelsmooth? Would the fill aesthetic clash with labels here?
  4. The vignettes need some work. Now is a good time to have some fun creating pretty examples.
  5. The tests cover all the code paths, but I do worry they don't explore edge cases that might cause problems. Some exploration of trying to find inputs that unexpectedly break would be helpful, but this might be difficult to do.

I will have a look at the checklists etc as you suggest. I'm sure this will be helpful.

@teunbrand
Copy link
Collaborator Author

Did you want to include any other rich text tags?

I'm sure there are a few that are feasible to implement, such as <code>, and <small> and <big> seem also straightforward enough. It is currently possible to get the same effect of these tags to work with the <span> tag, so I don't see this as being very urgent. Another thing I'd like to avoid is supporting more rich text options than {gridtext}, as we have a text placement package and not a text markup package and the options now are sufficiently rich to cover most cases.

other than underline

Getting underlines to work for single line, curved text might actually not that much of a problem, since we're retrieving underline positions anyway in font_info_gp(). It is more of a challenge for multi-line or mixed font text if every line is to be underlined. What we might be able to do is allow offset = "underline" in addition to the offset = unit(1, "cm") we allow now, which would then place the text accordingly.

Do we want to put the ribbon back into geom_textsmooth and geom_labelsmooth?

I don't really have a strong opinion on this, it'll likely mean giving geom_textsmooth() a custom draw_panel() method, no? If we decide not to put the ribbon in, I think this should be made clearer in the documentation. We can already place the text at +/-SE positions as follows, though using stage() might be too advanced for beginners.

library(geomtextpath)
#> Loading required package: ggplot2

ggplot(Indometh, aes(time, conc)) +
  geom_point(alpha = 0.1) +
  geom_smooth() +
  geom_textpath(
    aes(y = stage(conc, after_stat = ymax)),
    label = "Indomethacin",
    stat = "smooth", text_only = TRUE
  )
#> `geom_smooth()` using method = 'loess' and formula 'y ~ x'
#> `geom_smooth()` using method = 'loess' and formula 'y ~ x'

Created on 2022-01-09 by the reprex package (v2.0.1)

Would the fill aesthetic clash with labels here?

Yes definitely. If we do decide to put the ribbon back, we might have a ribbonfill and labelfill that default to fill, like we've disentangled the text from the line colours. Besides documenting this, I don't think it would be too much of a problem. On a tangential note, should we change textcolour/linecolour to text_colour and line_colour?

Some exploration of trying to find inputs that unexpectedly break would be helpful, but this might be difficult to do.

We could exhaustively test binary or character inputs to the the text parameters in combination with every geom and manually inspect that the plots look as expected. However that might still leave some blind spots and I don't think there is anything we can reasonably do to avoid this. This is why I suggested a pre-release announcement soliciting feedback, so that when people run into problems, they can let us know.

In addition, there is just so much depth to typesetting and text in general that I don't think we can feasibly cover everything. For example, font ligatures break in our package because they are encoded as separate glyphs, but really should be placed as a single glyph to work well and I don't know how we can know what the ligature characters for a particular font are. We also don't enjoy the regular font fallback heuristics because we lookup every glyphs in the font file.

@AllanCameron
Copy link
Owner

Hi Teun

I have added to the vignettes and rebuilt them. I'm quite happy with the introduction, aesthetics and polar co-ordinates vignettes. The "gallery" needs a bit of work. I don't know if you have any pretty examples standing by?

The other thing I thought of was that we should add an acknowledgements section in the readme. As well as thanking the ggplot team themselves, we should probably mention Clause Wilke for the richtext and Patrick Plenefisch for starting the ball rolling with the SO question (and pointing out some early issues). Anyone else?

Other than that, I still need to tweak the smoothing parameter (which I will get round to very soon) before being happy enough to declare a release.

You have more experience with CRAN submission than I do, so I don't know what their turnaround time is. If it's not too long, it might be reasonable to wait until post-CRAN before making an announcement and adding ourselves to the extension gallery?

It's nice to see the package getting some love on Twitter (including a retweet by Hadley himself). I don't have much of a Twitter presence, but I assume that we can get retweeted by a popular bot by adding the appropriate hashtag once we want to make an announcement?

@teunbrand
Copy link
Collaborator Author

Hi Alan,

I don't know if you have any pretty examples standing by?

No, I don't I'm afraid. I'm usually quite functional in my plotting. If we see nice examples of our package being used, we could ask people if they could share data and code maybe?

Anyone else?

Probably Thomas Lin Pedersen, for his work on the systemfonts/textshaping packages (in addition to ggplot2). It's just great that we can rely on these packages for all sort of textshaping business that I'd rather not be bothered by (e.g. kerning, text measurements, BiDi text, composite characters etc).

I don't know what their turnaround time is

In my experience (n = 1), the time to inspect and approve a new package was about a week. I had to resubmit a few times due to things that could have been caught using the checklist, so I highly recommend that. I'll run most of the checklist once we're satisfied, but I think some things run through the email of the maintainer (the {rhub} thing, I think), so I can ask you to do those things, or simply warn you that an actionable email is underway. Updating a package is usually faster than a week.

but I assume that we can get retweeted by a popular bot by adding the appropriate hashtag once we want to make an announcement

That's right, there is a bot (or multiple ones?) that just retweets #rstats posts. There is also the cranberries feed that automatically tweets new stuff posted on CRAN.

@AllanCameron
Copy link
Owner

OK, I've updated the smoothing mechanism. I would like a little more time to develop some examples for the gallery. Are there any other topics you think should be covered in the vignettes?

@teunbrand
Copy link
Collaborator Author

teunbrand commented Jan 11, 2022

There is no hurry to get this on CRAN, we can take the time we like. There are a few things vignette-wise I could think of:

  • Maybe we ought to rename the aesthetics vignette 'Aesthetics and parameters' and make sure we discuss a bit what all parameters do.
  • In the polar coordinates vignette, it would seem more natural to explain first that straight text in polar coordinates becomes curved, and afterwards that we can use the same principle for the axis labels.
  • I'm not sure if we should include something about how we are placing text? I'm thinking in the direction of a visual explanation of how we take the angle bisectors, use the min-max positions of letters to calculate the angle and such. It's also a good place where we can explain the curvature and how that may affect text placement. Potentially, this could also be a place where we note some of the font feature stuff, like bidi text or composite characters.

Besides vignettes, do you think you'd like to export your text smoothing algorithms as stat_* functions/layers? I'm not saying that this is necessary, but it might give people a feeling of control?

@AllanCameron
Copy link
Owner

Maybe we ought to rename the aesthetics vignette 'Aesthetics and parameters' and make sure we discuss a bit what all parameters do.

I was going to suggest "Aesthetics and scales", since I added a bit about the scale_vjust_ functions into this vignette. It would still be appropriate to discuss other parameters in there though, and I'm not too fussy about the name

In the polar coordinates vignette, it would seem more natural to explain first that straight text in polar coordinates becomes curved, and afterwards that we can use the same principle for the axis labels

This seems reasonable - I'll make the changes

I'm not sure if we should include something about how we are placing text?

I had the same thought, but decided to simply describe the general makeContent mechanism in a "How does it work?" section in the introduction vignette. I didn't discuss the specifics of the geometric calculations, since I think a full exposition would be more like a journal article than a vignette. Feel free to expand on it if you think that's worthwhile, but I'm not convinced many folk would read it.

do you think you'd like to export your text smoothing algorithms

Probably not. In the end I decided I was reinventing the wheel with the noisy path smoother and just wrapped stats::smooth.spline, with x and y as independent variables of path length. This has a predict method, so could be coerced to work with geom_smooth. The corner smoother would make an interesting stat, but it seems a bit of a random function to export as part of this particular package.

I've updated the readme with the acknowledgements we discussed above, so I think we only really need a bit of work on the vignettes now.

@AllanCameron
Copy link
Owner

I have added a new plot to make as the centrepiece of the gallery vignette.

It uses some made-up data that is probably a bit long for a visible code block. Do you think I should I add this data as a long code block in the vignette, have it as a demo data object in the package, or just leave it out altogether?

@teunbrand

This comment has been minimized.

@AllanCameron
Copy link
Owner

Hi Teun

I found that there were some unused images that could be removed from the man/figures/ directory. Along with a couple of cuts to the readme, this seems to bring us out of the 5MB danger zone.

One thing that has been bugging me is that the vjust doesn't seem to match geom_text, as we saw in #55 . I have spent a bit of time looking at this over the weekend, but can't easily see where we've gone wrong. I'm sure I can fix it given time, but do you think this is something worth investing time in ahead of a CRAN submission?

@teunbrand
Copy link
Collaborator Author

Hi Alan,

Great that you found some places to reduce size!

do you think this is something worth investing time in ahead of a CRAN submission?

I also tried looking into this but it was also not very clear to me where we deviate from grid's vjust. It seemed a multiplicative error and not a constant and AFAIK we don't multiply the vjust anywhere and just use the x-height as a constant offset. We can give ourselves some time to figure it out if you like.

@AllanCameron
Copy link
Owner

AllanCameron commented Jan 16, 2022

In case it's helpful, we should be aware that the number of pixels per inch should be 96 and not 72. I double checked this by measuring how many pixels the bottom of a letter x moves by when shifted down one inch:

library(grid)
library(geomtextpath)

x11()
grid.newpage()

tg <- textGrob(
        label   = "x",
        x       = unit(3, "in"),
        y       = unit(5, "in"),  
        vjust   = 0.5,
        gp      = gpar(fontsize = 100)
      )

grid.draw(tg)
ras <- dev.capture()

dev.off()


y_5 <- max(which(!apply(ras, 1, function(x) all(x == "white"))))

x11()
grid.newpage()

tg <- textGrob(
        label   = "x",
        x       = unit(3, "in"),
        y       = unit(4, "in"),  
        vjust   = 0.5,
        gp      = gpar(fontsize = 100)
      )

grid.draw(tg)
ras <- dev.capture()

dev.off()


y_4 <- max(which(!apply(ras, 1, function(x) all(x == "white"))))

y_4 - y_5
#> [1] 96

I mention this because some of the functions appear to have resolution of 72. Of course, this is points per inch rather than pixels per inch, but we seem to be treating it is pixels per inch in some calculations.

@teunbrand

This comment has been minimized.

@AllanCameron
Copy link
Owner

Yes, though part of that is that the line spacing isn't quite right either. With 100 point text I am getting 93 pixels of vertical white space between the "x"s in "x\nx" in a textGrob versus 83 pixels in a textpathGrob

@AllanCameron
Copy link
Owner

AllanCameron commented Jan 16, 2022

It does seem to be off by a 1 - (92 / 72) = 0.25 textheights

Although if you make the font really large, you'll see the correct ratio is nearer 0.79 than 0.75, which means the 72/96 ratio is probably coincidental

Though the max_ascend/lineheight in the font metrics is suspiciously close at 0.787

@AllanCameron
Copy link
Owner

AllanCameron commented Jan 17, 2022

I seem to have an empirical solution to the vjust problem. The changes I made are

  1. When calling get_offset and get_smooth_offset inside place_text, we multiply offset by 72/96
  2. We need a correction factor for lineheight in text_shape, partly to compensate for this, and partly because the lineheight seems different between geom_text and geom_textpath. This correction factor is about 1.45.

There are a few things that bug me about this:

  1. It's almost but not quite perfect.
  2. I haven't found out why we need to multiply offset by 72/96
  3. I don't know why there is an apparent multiplicative difference (of about 1.1) between the line spacing in shape_text and geom_text which results in having to apply this "magic constant" of ~1.45 (96/72 * ~1.1)

Anyway, the modifications above are certainly an improvement on the current vjust. The readme still looks good on knitting, but the tests fail due to complaints about offset measurements. Before bothering to change the tests and commit I wondered what your thoughts were on how it looks, and whether we should pursue it any further than this. My impression from looking at the use cases on Twitter and the blogosphere is that this is going to be good enough for all (or almost all) practical purposes.

library(geomtextpath)
#> Loading required package: ggplot2

p <- function(vjust, lab = "ctrl") {

  df <- data.frame(
  x = c(0, 1),
  y = 0,
  lab = lab
  )
  
  ggplot(df, aes(x, y, label = lab)) +
  geom_textpath(vjust = vjust,
                size = 4) +
  geom_text(
    data = df[1,],
    aes(x = 2),
    vjust = vjust, size = 4
  )
}

p(0)

p(-1)

p(1)

p(3)

p(3, "multi\nline")

Created on 2022-01-17 by the reprex package (v2.0.1)

@teunbrand

This comment has been minimized.

@AllanCameron
Copy link
Owner

Yes, I agree. I don't think I'll be able to leave this alone until I'm satisfied that I understand it. At least if there is some reason why we can't get it the same as geom_text (though this seems unlikely), it would be good to know why we can't.

@AllanCameron
Copy link
Owner

The vjust saga continues.

I have noticed something odd about textshaping::shape_text. It says in the docs with regards to y_offset column in the shape tibble:

The y offset in pixels from the origin of the textbox

Now, if that is the case, then surely keeping the same size of font but increasing the resolution should increase the y_offset in pixels proportionately? But it doesn't:

x72  <- textshaping::shape_text("x", vjust = 0.5, size = 40, res = 72)
x144 <- textshaping::shape_text("x", vjust = 0.5, size = 40, res = 144)

x72$shape$y_offset
#> [1] -18.10938

x144$shape$y_offset
#> [1] -18.10156

Presumably then, these measures are in points rather than pixels. I'm only noting this fact here because I think it will be helpful in solving the vjust issue.

Created on 2022-01-18 by the reprex package (v2.0.1)

@AllanCameron
Copy link
Owner

I've added a CRAN comments file and a checklist. I have ticked off everything in the checklist up until the actual submission. Are you happy that I go ahead? The only note I am getting in the checks is the new submission note.

@AllanCameron
Copy link
Owner

Well - I've submitted to CRAN, so I guess now we wait...

@teunbrand
Copy link
Collaborator Author

Yes that check should be there, there is no way to get rid of it until it is on CRAN. If you feel ready to submit to CRAN, go ahead! 🎉

@AllanCameron
Copy link
Owner

Well, we get a straight up fail on the incoming tests. The Windows build fail I think is a problem with the CRAN build machine, because it says "warning: no fonts were detected on your computer" and fails on trying to recreate the examples.

The Debian build fails because it doesn't manage the BiDi tests - these will need skipped on Debian. However, I have asked CRAN to look into the Windows build fail, because it doesn't seem right to me.

@teunbrand
Copy link
Collaborator Author

That is frustrating, the point of the rhub checks was to minimize surprises such as these. But I'm sure we'll be able to adapt.

@AllanCameron
Copy link
Owner

Ah, I have tracked the problem down to systemfonts. It doesn't detect any fonts on the development build of Windows. It is currently failing its CRAN checks

I submitted an issue on the systemfonts GH page, and Thomas Pedersen got back to me saying that he also thinks there is a problem with the CRAN Windows build, since this problem can't be replicated anywhere else. He has raised it with CRAN and is waiting to hear back from them.

Is it worth exploring dropping this dependeny? I think we are using it purely for lineheight and ascent measurements at the moment.

@teunbrand
Copy link
Collaborator Author

Right that makes sense then. I don't think it is worth dropping the dependency, we are also using it to translate glyph ids between font index and actual glyphs, so without it also {textshaping} becomes unusable. I think the action for us at the moment is sit tight, hope than CRAN repairs its windows machines and resubmit when they have.

@AllanCameron
Copy link
Owner

AllanCameron commented Jan 20, 2022

The other alternative that I'm trying is to include a free font in the inst directory, and write an .onLoad function that checks whether there are any system fonts detected, and if not calls systemfonts::register_font so that there is always a backup font available. In addition to maybe getting us past this initial problem, it might make the package more robust to similar problems in the future:

.onLoad <- function(libname, pkgname) {
  
  # Provide fallback font in case unable to detect system fonts
  if(nrow(systemfonts::system_fonts()) == 0) {
    systemfonts::register_font("fallback",  "./inst/NimbusSanL-Reg.otf")
  }
}

@teunbrand
Copy link
Collaborator Author

That is a pretty good idea, and if we could include a font that has hebrew glyphs, we'd have a fallback for the bidi tests as well! The only thing I'm mildly worried about is that we in essence get a similar problem as with the test for the autodocumentation of the ... argument: that within the context of testing, we can't access any files included in the package.

@AllanCameron
Copy link
Owner

we can't access any files included in the package.

I think I've figured out how to do this - hrbrmstr does it in hrbrthemes so I've copied his method.

@AllanCameron
Copy link
Owner

Yes. This works. We put the font in directory inst/font then do

.onLoad <- function(libname, pkgname) {

  nimbus <- system.file("font", "NimbusSanL-Reg.otf", package = "geomtextpath")

  # Provide fallback font in case unable to detect system fonts
  systemfonts::register_font("fallback", nimbus)

}

Note that I was going to use DejaVu, but the fontfile is 750KB and pushes us over the 5MB limit. If you want to find a decent free font that's <500KB, then please go ahead. I don't think it will ever be used - it's really just there as a fallback for systemfonts.

@AllanCameron
Copy link
Owner

Nope, registering the font is no good. We still get a "no fonts detected" warning. Frustrating.

@teunbrand
Copy link
Collaborator Author

teunbrand commented Jan 20, 2022

Lato is a font that is in the <100kb range and can be redistributed under the Open Font License, but I that is useless if the approach doesn't work. CRAN can be frustrating sometimes, especially when there is no way to replicate their machines. I am also excited to get this on CRAN, but there might be nothing we can do until the systemfonts thing is resolved.

@AllanCameron
Copy link
Owner

I'm trying one more thing. I think as long as the "fallback" family is specifically named in glyph_info it should be OK. At the moment systemfonts tries to find "default", which it doesn't recognise. I thought it would fall back to the only registered font, but it doesn't. I think specifically naming the family as the default will help. However, this means we need to ensure that this only happens when there are no system fonts detected, otherwise the metrics will default to Nimbus when no font family is specified.

@AllanCameron
Copy link
Owner

Yes, this seems to be working. I'm down to three failed tests. One of them is BiDi, which seems to be due to the lack of the correct glyphs in my font, so should be fixable. The other two are failed flipping tests. It's really hard to debug because my only option is to run the win_dev check and wait 20 minutes to be E-mailed with the log file. The other issue is that the aesthetics and gallery vignettes fail to build - my guess is that this is because they use glyphs or fonts that aren't available.

It looks like we will be able to get round this with a bit of work - I'll try your font suggestion too.

@teunbrand
Copy link
Collaborator Author

I'd be OK to add those vignettes to the buildignore file, which means they'll still be shown on the website, but wouldn't show up in the R documentation. I think it is also fine to skip the failing tests on CRAN, as the issue seems to stem from some weirdness on their part. Its not ideal, but I know from GHA that it'll work on other people's systems, so I'm not too worried.

@AllanCameron
Copy link
Owner

It seems I need to remove the gallery vignette altogether, which doesn't bother me - there were only two examples on it, and I was only happy with one of them. It will remain on the pkgdown site anyway as long as we don't rebuild, and we have the option of reinstating it once we are safely on CRAN.

Anyway, I am now getting only the "new submission" note on the Windows Development build, and the other CRAN checks are passing, so I am committing these changes to GH and then will resubmit.

@AllanCameron
Copy link
Owner

Success! I have submitted to CRAN and it has passed the incoming checks. Now we just need to wait to hear back about the millions of things we need to change following the manual checks!

@teunbrand
Copy link
Collaborator Author

teunbrand commented Jan 21, 2022

Great news, well done Allan on fixing the issues! First barrier down, one left to go 👍 If we're lucky, we'll get a reviewer that points out a few spelling mistakes or 'make sure every exported function documents its return value' or something of the sort. If we're very lucky, we'll have no comments!

@AllanCameron
Copy link
Owner

I just got this E-mail from CRAN -

Thanks,
on its way to CRAN.

Best,
Julia Haider

Is that an acceptance, or do they mean it's on its way for further checks?

@AllanCameron
Copy link
Owner

Hmm. Seems that was an acceptance. We now seem to be listed on CRAN, with publication date of today, though at the time of writing the binaries and checks aren't uploaded, so it can't yet be installed with install.packages

@teunbrand
Copy link
Collaborator Author

Oh nice, we got the good email straight from human review! 🎉 It takes maybe 1 or 2 days for the binaries to be build and uploaded, but the cranberry feed already took note. I guess it is time to roll out an announcement? Maybe add to the extension gallery?

@AllanCameron
Copy link
Owner

Great! I think we would get maximum momentum on Twitter if folks can already download the binaries from CRAN before we do so. Would you like to do the Twitter announcement? Perhaps with a collage of your favourite example images?

I don't see any reason why we can't go ahead and add it to the ggplot extension gallery straight away though. Again, a nice example is needed for the thumbnail. The spiral and the iris density plot seems to have become our "iconic" images. Although I'm not sure either is my favourite, the iris density plot tells at a glance what the package is all about. Perhaps just picking nicer colors for it? I don't mind doing that.

Congratulations to us both! A very successful and most enjoyable collaboration.

@teunbrand
Copy link
Collaborator Author

Yes congratulations from me as well! I too enjoyed the collaboration, it is much nicer than maintaining a package solo as somebody who also is familiar with the code base can think along and bounce ideas back and forth. I'll prep a thread for when the binaries drop, we could perhaps use the same image from the gallery as posterchild on twitter. The densities are nice, but I think people aren't too keen on the iris dataset anymore. Maybe we could use the palmerpenguins data? I like the spiral too, but it demonstrates what the package can do, not how it will likely be useful for data visualisation.

@AllanCameron
Copy link
Owner

Well, install.packages now works on my machine (builds from source)

@teunbrand
Copy link
Collaborator Author

Very satisfying moment isn't it?

@AllanCameron
Copy link
Owner

How about this for a gallery thumbnail using the Palmer's penguins data?

image

I like it, but I don't love it. This is the code:

library(geomtextpath)
library(palmerpenguins)

ragg::agg_png(width = 350, height = 300, bitsize = 16)
 ggplot(penguins, aes(bill_depth_mm, color = species)) + 
   geom_density(aes(fill = species), alpha = 0.3, color = NA) +
   geom_textdensity(aes(label = species, hjust = species), size = 7, 
                    vjust = -0.5, linewidth = 1,
                    text_smoothing = 55) + 
   facet_grid(species~.) +
   scale_fill_manual(values = c("#fe9d52", "#3c97da", "#2a8b53")) +
   scale_color_manual(values = c("#fe9d52", "#3c97da", "#2a8b53")) +
   scale_hjust_manual(values = c(0.9, 0.4, 0.55)) +
   theme_void() +
   theme(legend.position = "none",
         strip.text = element_blank(),
         axis.line.y.left = element_line(),
         axis.line.x.bottom  = element_line(),
         axis.text.x = element_text(vjust = -4),
         axis.ticks.length.x = unit(2, "mm"),
         axis.ticks.length.y.left = unit(1, "mm"),
         axis.ticks.x = element_line(),
         axis.ticks.y = element_line(),
         plot.margin = margin(20, 20, 20, 20),
         )
 dev.off()

@teunbrand
Copy link
Collaborator Author

teunbrand commented Jan 25, 2022

It looks good but I'm also not in love. What do you think about the gapminder data?

Rplot001

library(ggplot2)
library(gapminder)
library(tidyverse)
library(geomtextpath)

fi <- systemfonts::font_info("Roboto Condensed", size = 8 * .pt)
ul <- fi$underline_pos

df <- gapminder %>%
  group_by(continent, year) %>%
  summarise(lifeExp = mean(lifeExp), pop = mean(pop), gdpPercap = mean(gdpPercap))

p <- ggplot(df, aes(pop, gdpPercap, colour = continent)) +
  geom_textpath(
    aes(label = continent,
        linecolour = after_scale(colorspace::lighten(colour, 0.3))),
    text_smoothing = 50, vjust = -0.1, 
    arrow = arrow(length = unit(2.5, "mm")), offset = unit(-ul, 'pt'),
    linewidth = 1, size = 7, family = "Roboto Condensed", gap = FALSE
  ) +
  scale_colour_manual(values = continent_colors) +
  scale_x_continuous(
    "Population",
    trans = "log10",
    breaks = 10^c(7, 7.5, 8),
    labels = scales::math_format(format = log10)
  ) +
  scale_y_continuous(
    "GDP per capita",
    trans = "log10",
    limits = c(1000, NA),
    breaks = 10^c(3, 3.5, 4, 4.5),
    labels = scales::math_format(format = log10)
  ) +
  theme_void() +
  theme(legend.position = "none",
        strip.text = element_blank(),
        text = element_text(family = "Robot Condensed"),
        axis.line.y.left = element_line(),
        axis.line.x.bottom  = element_line(),
        axis.ticks.length.x = unit(2, "mm"),
        axis.ticks.length.y.left = unit(1, "mm"),
        axis.ticks.x = element_line(),
        axis.ticks.y = element_line(),
        axis.text = element_text(),
        axis.title.y.left = element_text(angle = 90),
        axis.title = element_text(),
        plot.margin = margin(5, 5, 5, 5),
  )

ragg::agg_png(width = 350, height = 300, bitsize = 16)
p
dev.off()

@AllanCameron
Copy link
Owner

AllanCameron commented Jan 25, 2022

Yes, this looks better. Given that the plot is small, the text could maybe be larger - perhaps with a little smoothing. I like the arrowheads, though my preference would be to shorten them a bit. The concept and colours are excellent though. Maybe some nice hrbrtheme font?

@teunbrand
Copy link
Collaborator Author

Fair points! I've updated the plot / code in my comment above. Would you be happy with this?

@AllanCameron
Copy link
Owner

I think just a couple of tiny tweaks to improve the text positioning and an intermediate arrow size. I'm happy with this:

p <- ggplot(df, aes(pop, gdpPercap, colour = continent)) +
  geom_textpath(
    aes(label = continent, vjust = continent, hjust = continent,
        linecolour = after_scale(colorspace::lighten(colour, 0.3))),
    text_smoothing = 50, 
    arrow = arrow(length = unit(3.5, "mm")),
    linewidth = 1, size = 6.5, family = "Roboto Condensed", gap = FALSE
  ) +
  scale_colour_manual(values = continent_colors) +
  scale_x_continuous(
    "Population",
    trans = "log10",
    breaks = 10^c(7, 7.5, 8),
    labels = scales::math_format(format = log10)
  ) +
  scale_y_continuous(
    "GDP per capita",
    trans = "log10",
    limits = c(1000, NA),
    breaks = 10^c(3, 3.5, 4, 4.5),
    labels = scales::math_format(format = log10)
  ) +
  scale_vjust_manual(values = c(-0.2, -0.2, -0.4, -0.2, -0.1)) +
  scale_hjust_manual(values = c(0.5, 0.7, 0.5, 0.3, 0.3)) +
  theme_void() +
  theme(legend.position = "none",
        strip.text = element_blank(),
        text = element_text(family = "Roboto Condensed"),
        axis.line.y.left = element_line(),
        axis.line.x.bottom  = element_line(),
        axis.ticks.length.x = unit(2, "mm"),
        axis.ticks.length.y.left = unit(1, "mm"),
        axis.ticks.x = element_line(),
        axis.ticks.y = element_line(),
        axis.text = element_text(),
        axis.title.y.left = element_text(angle = 90),
        axis.title = element_text(),
        plot.margin = margin(5, 5, 5, 5),
        plot.background = element_rect(fill = "#FDFEFF")
  )

image

I have already forked the ggplot extension gallery, so I will go ahead and submit the PR if you like.

Our Windows binary is available on CRAN already, and in any case I think anyone can install from source, so if you want to go ahead and make the Twitter announcement, I think we're good to go!

@AllanCameron
Copy link
Owner

I have submitted a PR to ggplot extension gallery with the above image as thumbnail.

@teunbrand
Copy link
Collaborator Author

Thanks Allan! I'll post a thread on twitter after I'm done with work

@AllanCameron
Copy link
Owner

We're now on the ggplot extensions gallery

@teunbrand
Copy link
Collaborator Author

Nicely done! Thread on twitter here.

@AllanCameron
Copy link
Owner

Great work Teun. Love the penguins plot. You are a bit of a plotting artist!

@teunbrand
Copy link
Collaborator Author

Thanks, it was mostly this example but with a different dataset :) So I think we've taken care of everything related to releasing this package, so I think we can safely close the issue if we want to.

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