Facet grids not working properly for "hist" and "density" types

[grantmcdermott]

Two-sided formulas are being interpreted a one-sided interaction:

tinyplot::plt(~wt, mtcars, facet = cyl ~ am, type = "hist")

Update: Density plots have their own problems. This time, we at least get a grid layout (titles on the RHS), but the arrangement is wrong (look the mistaken titles).

 tinyplot::plt(~wt, mtcars, facet = cyl ~ am, type = "density")

For comparison, this is what the correct grid arrangement should look like:

tinyplot::plt(~wt, mtcars, facet = cyl ~ am)

^{Created on 2024-08-27 with reprex v2.1.1}

[zeileis]

I think that the formula processing itself works correctly because it identifies the right variables in the right parts. The problem is probably that interacting everything to a single facet factor with special attributes is probably not the best interface because this has to be taken apart later again. Maybe we should rethink this design?

In any case, the facet in your example seems to be set up as intended before calling tinyplot.default:

attributes(facet)
## $levels
## [1] "0~4" "1~4" "0~6" "1~6" "0~8" "1~8"
## 
## $class
## [1] "factor"
## 
## $facet_grid
## [1] TRUE
## 
## $facet_nrow
## [1] 3

So the problems seem to happen later on in the code. Will try to have another look later.

[zeileis]

OK, the problem seems to be that collecting the facet in the datapoints and then applying split() drops the extra attributes facet_grid and facet_nrow. Preserving them fixes the issue. The question is just how to do this best.

One approach might be to make sure that formula is turned into a factor and then use the class c("facet", "factor") and supply a split.facet() that preserves the attributes. This would just need to be locally available and probably not exported by the package.

I'm still not fully convinced, though, that this is the best way forward (as mentioned previously). Maybe we should consider a more explicit declaration of facets rather than an interaction factor with attributes.

Two things I noticed while going through the code:

• Is the facet_grid attribute even needed? It seems that after checking that isTRUE(facet_grid) most code lines just use facet_nrow afterwards. So wouldn't is.null(facet_nrow) be sufficient? I'm not sure whether there are facet_nrow occurrences with facet_grid = FALSE though.
• It might be good to adapt get_facet_fml() to use the new tinyformula() and tinyframe() helper functions I added for the formula processing.

[grantmcdermott]

Thanks for looking into this @zeileis. I'm going to pretty tied up until mid-September, so I won't be able to give this much thought in the short-term unfortunately. Happy for you to propose any changes, though!

[zeileis]

I've filed a minimal PR now that fixes the problems for hist (facet attributes were not preserved at all) and density (only facet_grid but not facet_nrow was preserved). I think it's a sufficient workaround for now but when we have finished the modularization of the code, we should revisit the question of how to specify the facet information more generally.

[vincentarelbundock]

Thanks @zeileis PR looks great.

It's the start of semester here, so I probably won't have time for more modularization immediately. But I agree that a more elegant permanent solution should probably wait until that is done.