Passing Options

[ptoche]

1. Great package, love it! My favourite has to be theme_excel() :+1:
2. You can pass, say, theme_wsj(20) in a manner analogous to theme_bw(20), but this doesn't work for theme_stata() because base_size is the second option. I suggest changing that so one can type theme_stata(20) without having to think too hard. Thanks!

[jrnold]

1. Thanks!
2. It might have been good if I had done that to begin with, but now I don't want to break any existing code by changing it. You can get around this by always using base_size as a named argument

theme_wsj(base_size = 20)
theme_stata(base_size = 20)

[ptoche]

No problem, it was just a thought.

The simplest would have been to change line 239 from:

theme_stata <- function(scheme="s2color", base_size = 11, base_family = "sans") {

to:

theme_stata <- function(base_size = 11, scheme="s2color", base_family = "sans") {

That would break if anyone is calling the function like this: theme_stata("s2color", 12, "sans") but you could have an if numeric check to see if the font size is being passed first or second, which would make the code backward-compatible. Anyway, just a thought. Cheers!

[jrnold]

It would be easy to change, but I still don't see the need to. If there's a lot of complaints I'd reconsider. If I were to change it, I'd deprecate the entire theme_stata function and make each scheme a separate theme, e.g. theme_stata_s2color, etc. I think I originally had scheme as a required argument, and only later made it optional.

In general, i don't like the style of correcting a user's "mistakes" by guessing their intent. The function should check for correctness and and throw an error if its wrong. Trying to correct for the user's intent results in lots of uncaught logical bugs which are even harder to track down than responding to an explicit error message.

[ptoche]

Sure, it's not a problem. The last point was not to correct a user 'mistake' but to make it possible to use the theme_stata() function in the same way as theme_bw() from ggplot2 and in the same way as you wrote it. Enough said. Thanks and cheers for now.

[jrnold]

You're right that I should have been more consistent in the ordering of the arguments in the themes, and I'm kicking myself for not doing that in the first place. I just don't think it's big enough to break any existing code of other people (to whatever extent anyone is using it). But it's in my head now, so maybe my opinion will change. Thanks again!

[ptoche]

Oh please don't kick yourself, it never was worth the long comments I made. I enjoy ggthemes very much. I used to spend a lot of time tweaking styles, but now I'll just pull one style from your existing ones. (to me the main point of using theme_stata() is to show Stata users they can easily switch to R!)

[jrnold]

Rethought this one. Since it was always optional, option order wasn't guaranteed.

[ptoche]

:+1: :+1: :+1:

[ptoche]

I noticed theme_solid() also has a different order of options. The effect of theme_solid(20) is not error but to pass the option to fill with the result that the background color changes to blue.

[jrnold]

Hmm, on one hand since the font family and font sizes are ignored, there shouldn't be any options. But if it is used as a parent theme, they should exist.

[ptoche]

Another comment in passing, of utmost triviality, you have named two files theme-foundation.R and theme-solid while the other theme files are named e.g. tufte.R. Perhaps it will be easier to read the code if you name everything like the function, e.g. theme_solid.R, theme_tufte.R, etc.. Makes it easier to find your way around the R directory.

[jrnold]

I looked at theme_solid, and it makes no sense to include parameters that aren't used and in fact wouldn't be inherited. I'd rather throw an error than have those ignored.

[jrnold]

I just use grep :-)

[jrnold]

Rethought my rethinking. Made it so that base_size and base_family are the 1st two arguments to all user-facing theme_* functions. But, it's still good and safe style to always use argument names for all optional arguments.

[ptoche]

agreed about throwing an error! Thanks for your hard work :+1: :+1: :+1: