Spineplots and spinograms for factor y-variables

[zeileis]

Fixes #2

This PR will eventually support spineplots (factor ~ factor) and spinograms (factor ~ numeric) using the type_ infrastructure from the epic #222

There is already some good progress. For example, you can do:

library("tinyplot")
 aq = transform(
   airquality,
   Month = factor(Month, labels = month.abb[unique(Month)]),
   Hot = Temp > median(Temp)
 )
 tinyplot(Hot ~ Wind, facet = ~ Month, data = aq, type = type_spineplot(), breaks = 4)

ttnc <- as.data.frame(Titanic)
tinyplot(Survived ~ Sex, facet = ~ Class, data = ttnc, weights = ttnc$Freq, type = type_spineplot())

But as you can see the axis labeling is not great and the by handling does not work properly, yet. I will ask various questions, mostly to you Vincent @vincentarelbundock, I'm afraid. But I'm optimistic that we can sort out the details.

tbc

[zeileis]

First questions:

• The xaxs = "i" and yaxis = "i" are still not passed to the right place. Do I need to add these as explicit arguments somewhere else?
• Some arguments that I set in the data_ function I want to access in the draw_ function. What is the best way to do that? Currently I include these in the return value (e.g., https://github.com/grantmcdermott/tinyplot/blob/spineplot/R/type_spineplot.R#L156) and then fetch them from the parent.frame() (https://github.com/grantmcdermott/tinyplot/blob/spineplot/R/type_spineplot.R#L21-L22). But that's probably not a good solution...

[vincentarelbundock]

* The `xaxs = "i"` and `yaxis = "i"` are still not passed to the right place. Do I need to add these as explicit arguments somewhere else?

What functions consume these arguments? With your current code, the variables themselves should already be available in the tinyplot() scope, but I don't see any call anywhere that uses these variables a inputs. Should they be passed to the facet drawing function, the window creating, or to draw_spineplot()? I think things are setup correctly in your type_spineplot() code. It's probably just a matter of carrying through as inputs to the proper functions.

* Some arguments that I set in the `data_` function I want to access in the `draw_` function. What is the best way to do that? Currently I include these in the return value (e.g., https://github.com/grantmcdermott/tinyplot/blob/spineplot/R/type_spineplot.R#L156) and then fetch them from the `parent.frame()` (https://github.com/grantmcdermott/tinyplot/blob/spineplot/R/type_spineplot.R#L21-L22). But that's probably not a good solution...

What "shape" do these arguments have? One simple option might be to use data_spineplot() to insert this info as new columns with idiosyncratic names into datapoints. Then, draw_spineplot() has access to that data and can retrieve the info directly.

[zeileis]

The xaxs and yaxs properties have to be set when creating the outer plot to which the types are then adding the actual content. So I think it needs to be passed to draw_facet_window() and then ultimately plot.window(). Is there some way to achieve this? Or do I need to add arguments xaxs and yaxs explicitly for tinyplot() and then pass them through everything?

Regarding the extra arguments: The most prominent case are the breaks for the spinograms. These are the points at which I split the numeric x variable into categories. I want to compute them on the entire data only once, that's why I put them in data_spineplot().

As they are neither a scalar, nor of length "n", I did not put them as a column in datapoints. But I could put them there if I pad with NAs. So it's technically possible but also not elegant.

You could argue that I ought to cut() the x into categories in the data_spineplot() function already. And that's probably a good idea. But I would still need to pass along the underlying breaks because I need these for making nice axis labels.

[vincentarelbundock]

The xaxs and yaxs properties have to be set when creating the outer plot to which the types are then adding the actual content. So I think it needs to be passed to draw_facet_window() and then ultimately plot.window(). Is there some way to achieve this? Or do I need to add arguments xaxs and yaxs explicitly for tinyplot() and then pass them through everything?

If we think users will want to specify xaxs explicitly themselves in other plots, then we could add them to the main function. But if you think it's mostly an internal thing, then you could add:

xaxs = yaxs = NULL

just before data_type() is called. Then, your function overrides the default NULL value. draw_facet_window() can then be modified to accept the internal xaxs value, ignore it if is NULL, or act correctly if it is non-null.

As they are neither a scalar, nor of length "n", I did not put them as a column in datapoints. But I could put them there if I pad with NAs. So it's technically possible but also not elegant.

Here's one idea: In the main tinyplot function, just before calling type_data(), create an empty list called type_info. Then, data_spineplot() overrides that empty list with a named list of whatever you need in the drawing function. Finally, we modify the main tinyplot() function to pass type_info to type_draw().

Since every type_draw() function accepts ..., type_info will be ignored most of the time. But then custom types like yours have an easy mechanism to pass arbitrary data from type_data() to type_draw().

[zeileis]

Great, Vincent @vincentarelbundock, very useful. I've added the xaxs and yaxs arguments now - also to tinyplot() as they are standard par() arguments. Grant @grantmcdermott, let me know if you disagree and would not have exported them.

[zeileis]

And nice idea with the type_info, I've also added that now! :bulb:

I didn't do extensive tests, yet, but I think that tinyplot() and plot() now give the same output for factor ~ factor and factor ~ numeric! :tada:

Next I want to polish the faceted displays and then have a stab at handling by variables. For the facets I have two questions:

1. Is there a recommended way how to increase the margins between the displays? Because spine plots employ both the left and the right y-axis for labels, we need a little bit more space here.
2. Because for spineplots type_draw is drawing the axes rather than draw_facet_window: Can type_draw know whether it is in facet on the very left or very right and at the top or at the bottom, respectively? Then, we could draw fewer axes, if we want.

[vincentarelbundock]

I didn't do extensive tests, yet, but I think that tinyplot() and plot() now give the same output for factor ~ factor and factor ~ numeric! 🎉

Very, very cool!

1. Is there a recommended way how to increase the margins between the displays? Because spine plots employ both the left and the right y-axis for labels, we need a little bit more space here.

I don't know about margins. Paging @grantmcdermott

2. Because for spineplots `type_draw` is drawing the axes rather than `draw_facet_window`: Can `type_draw` know whether it is in facet on the very left or very right and at the top or at the bottom, respectively? Then, we could draw fewer axes, if we want.

I see a facet_window_args object in the main tinyplot function with a bunch of information in it. I bet if you pass this to type_draw(), you could match it to ifacet which gives you the index of the current facet.

[grantmcdermott]

Very exciting 🚀

1. Is there a recommended way how to increase the margins between the displays? Because spine plots employ both the left and the right y-axis for labels, we need a little bit more space here.

Yes. That's the fmar parameter, which can be accessed/set either: 1) temporarily as part of the list passed to tinyplot(...., facet.args = list(fmar = xx)), or 2) permanently via tpar(fmar).

Reading and typing quickly on my phone, so I hope I didn't misunderstand the question. I'll be able to look properly in an hour or so.

Edit: Details and default values here. https://grantmcdermott.com/tinyplot/man/tpar.html#additional-graphical-parameters

[zeileis]

Thanks, Grant. Then I see two ways of setting this:

1. We include facet.args in the fargs list for type_data so that it can be modified and subsequently passed on to draw_facet_window.
2. We just call tpar(fmar = ...) within the type_data function.

2 is leaner but I guess 1 is cleaner?

[vincentarelbundock]

Yeah, I don't see a good reason to keep away too many things from type_draw(). Seems like a general design.

[grantmcdermott]

RE: facet margin adjustments. Another option would be to check for type=='spineplot' and then do an automatic adjustment similar to what we do for other special cases here: https://github.com/grantmcdermott/tinyplot/blob/068b431302e08a55afedae9bff40d7cf03937db6/R/facet.R#L127-L152

E.g. In the last bit of the above code chunk, we subtract 0.5 lines from the fmar values if the plot frame is turned off (to reduce unnecessary whitespace between the individual facets).

Summarising: maybe we just try adding the following below line 152?

if (type == "spineplot") fmar = fmar + 1    # or however many lines you want to increase by

[zeileis]

Thanks for the advice, as usual very helpful. I now did the following:

• Avoid any type == "spineplot" to keep type processing as modular as possible.
• Pass facet.args to type_data so that type_spineplot can increase the default fmar.
• Pass facet_window_args to type_draw so that the axis(4) is only drawn in the right-most panel in each row.
• For this I added a new helper function is_facet_position (in facet.R) which can determine whether the current facet panel is on the "left" or the "right" and at the "top" or the "bottom" of the facet grid. Maybe we want to leverage this in other types as well?
• I added an interpretation of xaxt/yaxt to type_spineplot although I had to adjust their meaning a little bit because the axes are non-standard.

[grantmcdermott]

For this I added a new helper function is_facet_position (in facet.R) which can determine whether the current facet panel is on the "left" or the "right" and at the "top" or the "bottom" of the facet grid. Maybe we want to leverage this in other types as well?

Thanks @zeileis, I'll take a look. Would this supplant (duplicate?) the existing logic that we use here for only drawing axes of "outer" facets if the plot frame is turned off?

https://github.com/grantmcdermott/tinyplot/blob/068b431302e08a55afedae9bff40d7cf03937db6/R/facet.R#L38-L39 and https://github.com/grantmcdermott/tinyplot/blob/main/R/facet.R#L250-L265

[zeileis]

Thanks, Grant, I overlooked that feature. Why is that logic only applied if there is no frame.plot? Shouldn't this be disentangled? This would be helpful in general I guess. But for spineplots in particular because I don't have a standard plotting region so that frame.plot has to be treated differently/

For spineplots, at the moment, I always repeat axis 1 and 3 but axis 4 is only shown for the last panel in a row. But I'm happy to adapt

[zeileis]

Summary

The type_spineplot() is pretty decent now. The main missing feature is by which I will try to tackle next. Some fine details of margins and legends in the case of facets (see above) can still be improved but are no show-stoppers, I think.

My latest additions are:

• flip is supported now (by actually flipping the split direction and not just swapping the variables)
• more granular control of axes/xaxt/yaxt so that frames around the rectangles can be switched off
• facet colors are now supported in the usual way - by deriving simple sequential HCL-based palettes within each facet

Examples

library("tinyplot")
ttnc = as.data.frame(Titanic)
tinyplot(Survived ~ Sex | Class, facet = "by", data = ttnc, weights = ttnc$Freq, type = type_spineplot(),
  palette = "Dark 2", facet.args = list(nrow = 1), axes = "t", lwd = 5)

tinyplot(Survived ~ Class | Sex, facet = "by", data = ttnc, weights = ttnc$Freq, type = type_spineplot(),
  palette = "Dark 2", facet.args = list(ncol = 1), axes = "t", lwd = 5, flip = TRUE)

Problems

Legend symbols: Note that I have to set lwd = 5 in order to produce thick lines in the legend. It would be better to create filled rectangles there. Can I modify the type_draw function to achieve this?

Colors: Often one would want to select a single set of colors coding the levels of the y-variable like this:

Users transitioning from plot() would expect the following code to work but it leads to an error:

p = palette.colors(3, "Pastel 1")
tinyplot(Species ~ Sepal.Width, data = iris, breaks = 4, type = type_spineplot(), col = p)
## Error: `col` must be of length 1 or 1.

For now, I have worked around this by giving the type_spineplot() function another col argument but this isn't ideal. Maybe we would have to special case this once type_spineplot() is an official plot type?

tinyplot(Species ~ Sepal.Width, data = iris, breaks = 4, type = type_spineplot(col = p))

[grantmcdermott]

Whoa, these look great. I'll try to do a proper review tomorrow. (@vincentarelbundock please feel free to jump in first if you have time.) Really excited to see this long-standing issue nearing a resolution!

[zeileis]

Honestly, I wasn't sure whether we would really get here because there were so many special cases in the old monolithic code :see_no_evil:

Also I expected the modularization to be even more complicated. But Vincent's trick of passing a lot of arguments to the workers which can then overwrite them via listenv() is really net. I hadn't seen this before. :bulb:

[vincentarelbundock]

I hadn't seen this before. :bulb:

Me neither 😭

[zeileis]

How did you come across this idea? Was this Grant's input? (I didn't follow the details about the initial discussion of the modularization.)

Bonus question: The standard design for type_draw is to cycle through ever facet level within each by level. For spineplots I would need to draw all by levels simultaneously within a given facet level. In this case I would only draw anything if iby == 1L but I would need to get the relevant subset of the data. I guess I could piece it together from data_by but I wondered whether you see a more elegant approach?

[vincentarelbundock]

No, I just started by returning a bunch of arguments in a list and reassigning them. Then, I got lazy and used list2env() as a hack. Only later I realized it was kind of a neat trick.

Don't have a great idea now, and I'm not going to be able to concentrate on this for a few days at least since it's holiday here. Sorry!

[zeileis]

That's perfectly fine! I should really be doing other things as well (not vacations unfortunately). So I'll wait for Grant's feedback first and then return to handling the by variable later. Enjoy the vacations.

[grantmcdermott]

Sorry, I haven't had time to review this properly. I also have to head out of town now... but I just pushed a simple tweak (workaround) that gives square legend symbols.

pkgload::load_all("~/Documents/Projects/tinyplot")
#> ℹ Loading tinyplot

ttnc = as.data.frame(Titanic)

tinyplot(Survived ~ Sex | Class, facet = "by", data = ttnc, weights = ttnc$Freq, type = type_spineplot(),
         palette = "Dark 2", facet.args = list(nrow = 1), axes = "t")

It's a bit hacky and I'll also flag that the bespoke coloring override here means that we don't match the offset black correctly. For example, see the legend key for "1st" is darker than the plot region here.

tinyplot(Survived ~ Sex | Class, facet = "by", data = ttnc, weights = ttnc$Freq, type = type_spineplot(),
         facet.args = list(nrow = 1), axes = "t")

More generally, I need to think about the best way to pass arguments like col, lty, lwd etc. back and forth between the type_spineplot() constructors and the other drawing arguments. (I know that this is tricky b/c of spineplot's unique axes and colouring requirements, which are currently done "just in time" as part of draw_spineplot().

[zeileis]

Grant, thanks for this! Some comments and thoughts below. Nothing urgent, so feel free not to respond any time soon...

• I did the special casing of the default black (#000000FF) because (a) to match the default colors of plot()/spineplot() in base R and (b) because starting from black makes the sequential palette rather dark.
• Currently the color computations are indeed done just in time in type_draw but I could easily move them to type_data.
• However, as far as I can tell this would still not enable me to pass the right arguments to draw_legend, or am I overlooking something?
• In general, I think it would be good give the type constructors some control over the legend as well. Either via type_data or possibly via an additional type_legend or so, if necessary.
• It would also be desirable to get rid of most of the special cases for certain types, especially in draw_legend. For the basic type = "p", "l", "o", and friends such special cases are ok IMO but everything else would ideally be in the type functions.

[grantmcdermott]

However, as far as I can tell this would still not enable me to pass the right arguments to draw_legend, or am I overlooking something?

I don't think you're missing something. Or, at least, I didn't see how to do it either. Speaking of which...

In general, I think it would be good give the type constructors some control over the legend as well. Either via type_data or possibly via an additional type_legend or so, if necessary.

Yeah, this is a great shout-out. I don't know if we (collectively) have the time to fix this before the next release... which I was hoping to push through within the next week or two, once this gets merged. But doing so would greatly simplify / negate some of the legacy workaround that we've carried through from pre-modularization. (Basically, just +1 to your final bullet point.)

[zeileis]

OK, good. Then let me see when I get round to implementing the by variable for the spineplots. I'm afraid not within the next week - I'm also not sure, yet, how to best set it up (see above).

After that we can do a release and subsequently try to implement a more modular legend.

[grantmcdermott]

Sgtm. I'll take a stab at it too and will obviously let you know if I make progress. I have some ideas for moving the constructor functions over to spineplot_data, which may work...

[grantmcdermott]

Hi both. I've been experimenting with refactoring @zeileis's code in a way that should hopefully play nicely with our existing "by" logic (basically, trying to move all data construction parts to data_spineplot).

One tricky issue is that I need to pass some bespoke objects that I calculate in data_spineplot on to draw_spineplot, and I don't know how to do that with the current function factory logic. To make this a bit more concrete, say that my data_spineplot function looks like the following:

data_spineplot = function(...) {
  <do stuff>
  out = list(
    xmin = xmin,
    <more_stuff>,
    foo = 100   # special argument and value that I want to pass on to draw_spineplot
  )
  return(out)
}

How do I ensure that draw_spineplot has access to this foo = 100 argument that was part of the return list of data_spineplot()?

@vincentarelbundock this might be something you can answer given your in depth knowledge of the function factory logic.

[vincentarelbundock]

Up in this thread we came up with the type_info trick. It's just a list so you can store arbitrary named objects in it, and it gets passed from data_*() to draw_*(), so you can just retrieve it in the drawing phase.

[grantmcdermott]

Ah, I should have seen that. Thanks, will try!

[grantmcdermott]

@zeileis Quick HU that I've got a refactored version ready. (Refactored in the sense that it mostly just moves parts of your code around, but in a way that allows "by" without facets, etc.) I just want to do one or two more tests and then will share my version for you to test. Hopefully tomorrow.

[grantmcdermott]

Hi @zeileis,

I've finished my review+refactor in https://github.com/grantmcdermott/tinyplot/pull/233/commits/26e5635b1cd29b6246e0e07473c61b5d9881abef.

(Sorry to push these changes directly to your branch, but on balance I think that this is the simplest way to share my changes. We can always rewind this commit if there's something you don't like.)

Again, the "refactor" here was mostly me just moving parts of your existing code around. But FWIW the major differences from before are:

• All data transformations now happen in spineplot_data() rather than spineplot_draw(). This means that we can leverage standard tinyplot logic for handling "by" and "facet" grouping without worrying too much about bespoke spineplot requirements.
• Speaking of bespoke spineplot requirements, I use type_info() to pass any special arguments that we need from spineplot_data to sineplot_draw (e.g., the grouped-specific x(y)axlabels) and this seems to work well.
• In order to be consistent with the overall type_* logic that @vincentarelbundock introduced, I require that users explicitly pass special customization arguments like breaks and weights through type_spineplot(). Note that this is a "breaking" change from your original versions above, since e.g. the breaks argument in tinyplot(..., breaks = 4, type = type_spineplot()) will be ignored. Rather, you'd have to use tinyplot(..., type = type_spineplot(breaks = 4)).
• [Minor:] I tweaked to_hcl() and seq_palette() so that they are aware of alpha transparency.
• [Minor:] I added support for a type = "spineplot" convenience string using the default arguments.

From the (also newly add) examples in the docs:

pkgload::load_all("~/Documents/Projects/tinyplot")
#> ℹ Loading tinyplot

# "spineplot" type convenience string
tinyplot(Species ~ Sepal.Width, data = iris, type = "spineplot")

# Use `type_spineplot()` to pass extra arguments for customization
tinyplot(Species ~ Sepal.Width, data = iris, type = type_spineplot(breaks = 4))

p = palette.colors(3, "Pastel 1")
tinyplot(Species ~ Sepal.Width, data = iris, type = type_spineplot(breaks = 4, col = p))

# Grouped and faceted spineplots

ttnc = as.data.frame(Titanic)
tinyplot(
  Survived ~ Sex, facet = ~ Class, data = ttnc,
  type = type_spineplot(weights = ttnc$Freq)
)

# For grouped "by" spineplots, it's better visually to facet as well
tinyplot(
  Survived ~ Sex | Class, facet = "by", data = ttnc,
  type = type_spineplot(weights = ttnc$Freq)
)

# Fancier version. Note the smart inheritance of spacing etc.
tinyplot(
  Survived ~ Sex | Class, facet = "by", data = ttnc,
  type = type_spineplot(weights = ttnc$Freq),
  palette = "Dark 2", facet.args = list(nrow = 1), axes = "t"
)

# PS. If you really need to drop faceting for grouped spineplots, then at
# least add alpha transparency
tinyplot(
  Survived ~ Sex | Class, data = ttnc,
  type = type_spineplot(weights = ttnc$Freq),
  alpha = 0.3
)

^{Created on 2024-10-28 with reprex v2.1.1}

Overall, I think this PR is good shape now and I'm happy to merge if you're satisfied with the changes that I've added on top of. (Edit: Oh, and once we've added some snapshot tests!)

[grantmcdermott]

PS. Forgot to show in the examples, but flip = TRUE also works (again, leveraging the original code logic that you implemented.)

tinyplot(
  Survived ~ Class | Sex, facet = "by", data = ttnc,
  type = type_spineplot(weights = ttnc$Freq),
  palette = "Dark 2", facet.args = list(ncol = 1), axes = "t", 
  flip = TRUE
)

[zeileis]

Grant @grantmcdermott, thanks a lot for this, very much appreciated. And certainly ok to make the changes directly in the same branch

So far, I just had time to read your summary here and scroll through the examples. I will have to take a closer look at the code later. Just two quick comments:

• The by stuff will have to be special-cased for the spineplots, the overlays are not really useful. What I had planned to implement is essentially either spineplot(y ~ interaction(x, by)) or spineplot(y ~ interaction(by, x)) (just with better spacing and labeling). Haven't decided which one is really better, yet.
• The breaks=... argument is something that you change very often in practice when using spinograms. Not being able to use it as tinyplot(..., type = "spineplot", breaks = ...) but having to go through tinyplot(..., type = type_spineplot(breaks = ...)) might qualify as a dealbreaker, I'm afraid. The weights=... argument is also a nuisance but not quite as bad. Maybe there is some way to avoid this?

[grantmcdermott]

The by stuff will have to be special-cased for the spineplots, the overlays are not really useful. What I had planned to implement is essentially either spineplot(y ~ interaction(x, by)) or spineplot(y ~ interaction(by, x)) (just with better spacing and labeling). Haven't decided which one is really better, yet.

Ah, I see. I think that moving data construction to spineplot_data should still benefit this approach, as long as we handle the interaction(x, by) creation internally. I'll think on implementation too.

The breaks=... argument is something that you change very often in practice when using spinograms. Not being able to use it as tinyplot(..., type = "spineplot", breaks = ...) but having to go through tinyplot(..., type = type_spineplot(breaks = ...)) might qualify as a dealbreaker, I'm afraid. The weights=... argument is also a nuisance but not quite as bad. Maybe there is some way to avoid this?

Hmmm. Yes, it should be easy enough to revert. But I worry that this will introduce inconsistency into the overall API. For example, following #222 and #232 we have required that histogram breaks are passed through type_histogram():

tinyplot(Nile, type = type_histogram(breaks = 30)) # breaks work
tinyplot(Nile, type = 'histogram', breaks = 30)    # breaks ignored

So supporting breaks with type = "spineplot" would at a minimum imply that we have to revert this behaviour for type = "histogram" too. I guess we could make an exception and quietly allow breaks to be passed either way (e.g., so that both lines in the above code chunk produce the same outcome). But even then I worry that we're being inconsistent w.r.t. the special arguments that are used and passed by other types (say, amount for type = "jitter"). Perhaps I'm overthinking it. Curious to hear your thoughts as well @vincentarelbundock.

[vincentarelbundock]

The breaks=... argument is something that you change very often in practice when using spinograms. Not being able to use it as tinyplot(..., type = "spineplot", breaks = ...) but having to go through tinyplot(..., type = type_spineplot(breaks = ...)) might qualify as a dealbreaker, I'm afraid.

I think I'll disagree with this. It can be even shorter to use the proper form, because breaks is the first argument:

plt(x, y, type = type_spineplot(10))
plt(x, y, type = "spineplot", breaks = 10)

Even the full-length version is just 4 characters longer:

plt(x, y, type = type_spineplot(breaks = 10))

For the long term sanity of the interface, I think it's very important to be strict, and to stop adding new arguments to the main function. Sometimes it'll be a bit more verbose, but consistency of user interface is a super important feature, IMHO. And with completions with most IDE, this is a non-issue for most (though not all) users.

[vincentarelbundock]

I think this is actually quite important. And @grantmcdermott's histogram example points exactly where we'll end up: unending type-specific exceptions.

The main motivation for my refactor was to enforce a strict separate of code and documentation for each type. This was to ensure long term viability of the code base, but most importantly, sanity of the interface. If we start making lots of exceptions, I think it defeats the purpose.

[zeileis]

Thanks for the feedback. I understand all the points and agree that consistency is important.

I think what bothers me about it in this particular case that I have to move from

plot(y ~ x, breaks = fivenum(x))

to

tinyplot(y ~ x, type = type_spineplot(breaks = fivenum(x))

i.e., losing some consistency with plot(). But clearly we will not always be able to have consistency with both plot() and the new tinyplot() design.

[zeileis]

Thinking about this some more, I wondered whether it would make sense if tinyplot()

• looked at names(formals(paste0("type_", type))),
• compared it with names(dots),
• set up the tinyplot_type with the intersection of the arguments (rather than without any arguments).

[grantmcdermott]

i.e., losing some consistency with plot(). But clearly we will not always be able to have consistency with both plot() and the new tinyplot() design.

Yeah, I agree that these kind of edge case inconsistencies with plot are inevitable. The best that we can do is enforce internal consistency. One example that springs to mind is the fact that we always use bg/fill to refer to interior color and col to refer to outer color, whereas base is a bit inconsistent in my view (we don't support border for that reason).

Thinking about this some more, I wondered whether it would make sense if tinyplot()

• looked at names(formals(paste0("type_", type))),
• compared it with names(dots),
• set up the tinyplot_type with the intersection of the arguments (rather than without any arguments).

This is a reasonable proposal and I'll note that we already do something similar when passing the main plotting arguments here: https://github.com/grantmcdermott/tinyplot/blob/50903272b917085668c200a5eaa4e60c9039f9d2/R/facet.R#L208-L210

At the same time, I can think of some potential downsides, including argument duplication/clashes at the top and type-specific level. One simple example: type_spineplot() accepts a col argument to specifically handle the coloring "Problem" case that you describe at the bottom of this comment. But the main tinyplot() function obviously also accepts a col argument too, which has different restrictions and implications. So you'd end up with ambiguity.

Regardless of whether we ultimately decide to support the names(formals(paste0("type_", type))) approach or not, I think that's best left to separate discussion and is beyond the scope of this PR. Particularly since deciding to implement it would necessitate refactoring of other plotting types code and docs, beyond just spineplot.

If you agree @zeileis (or, disagree but can live with it!), then I'll wait on your review of my refactoring. We still need to implement your "by" interaction logic. But that should hopefully be the final step.

[grantmcdermott]

Pushed a few extra changes. including tests and two user-level features:

1. Special catch / logic for cases where x==by or y==by. The latter provides a more idiomatic tinyplot way of assigning consistent colours across y category.

tinyplot(
  Species ~ Sepal.Width | Species, data = iris, type = type_spineplot(breaks = 4),
  palette = "Pastel 1", legend = FALSE
)

2. Consistent with the original wishlist in #2, tinyplot now defaults to type_spineplot() if y is a factor variable. Using your Swiss labour examples from that issue:

data("SwissLabor", package = "AER")
tinyplot(participation ~ age, data = SwissLabor)     ## Factor ~ Numeric

tinyplot(participation ~ foreign, data = SwissLabor) ## Factor ~ Factor

[grantmcdermott]

I know that we haven't quite cracked the by (= interaction(x, by)) logic yet, but FWIW my feeling is that this PR is now good enough to merge.

(And by "good enough" I mean very good; I'm super happy with this spineplot functionality.)

We can/should revisit the spineplot by behaviour in a separate PR. But my goal is to submit a new tinyplot release to CRAN by the end of this week. It would be great for type_sineplot() to make it in... even if we plan on improving it down the line.

Any objections?

[vincentarelbundock]

I might release a tinytable soon. Great combo!

[zeileis]

Grant, thanks for these additions, super nice! And I agree that the additions are already useful enough to be worth merging/releasing. Do we need to put in some warning (or comment in the documentation) about which features are not yet supported?

[grantmcdermott]

Good call on the warnings. I'll add and then merge.

Thanks both, and especially you @zeileis for this excellent addition!

[zeileis]

Great, thanks! And feel free to put up a new issue about adding proper by support in the spineplots. I will definitely have a look at this but probably not in the next couple of weeks.