search

easystats / modelbased

:chart_with_upwards_trend: Estimate effects, contrasts and means based on statistical models
https://easystats.github.io/modelbased/
GNU General Public License v3.0
234 stars 19 forks source link

move visualisation_recipe base method to lower level #116

Open DominiqueMakowski opened 3 years ago

DominiqueMakowski commented 3 years ago

I'd like to re-use visualisation_recipe in other packages (e.g., it could be relatively easy to implement for correlation), which would require the move the base method definition to datawizard/insight (after modelbased makes it to CRAN first)

bwiernik commented 3 years ago

datawizard makes more sense to me. But why not see?

DominiqueMakowski commented 3 years ago

because it should be available without depending by other packages that don't depend on see (as visualisation_recipe only returns a list of info)

bwiernik commented 3 years ago

I guess I don't see the context where that would be useful though? It's a ggplot recipe

bwiernik commented 3 years ago

To elaborate, what is the context for using visualization_recipe() other than to produce a ggplot object, which would require the see ggplot2 dependency?

DominiqueMakowski commented 3 years ago

For end-users? not much, I suspect being able to make the plots using other software or inspecting the layers or reproducing the plot manually. For developers that use specific easystats packages as a dependency, it will allow them to get access to the "fabric" of the plots of that package (without having to depend on ggplot), that they could use in their examples/vignettes or customize etc. Most importantly, for us, it allows to separate the logic behind a plot from the rendering, and to save most of the code-producing logic next to the code of the function. Which makes it easier to code, debug and maintain.

bwiernik commented 3 years ago

I guess I don't really see the point of the see package if it doesn't house the plotting functions.

bwiernik commented 3 years ago

For either end users or developers, the recipes are still ggplot recipes, I don't think there is much value in them without ggplot, and I think there is a lot more simplicity for us to have the plot code together all in the see package than distributed around various places.

DominiqueMakowski commented 3 years ago

For either end users or developers, the recipes are still ggplot recipes

Currently, for the vast majority R-only users obviously yes. Though with a wider perspective, one has to take into account that there is an explosion of development of GG viz packages in other languages like python and Julia (which use an almost identical API), and multilanguage interoperability and joint-usage is one of the very likely future of data science (cf. recent pushes of rstudio on that front).

But that's just a possible side-benefit and not a main goal of what I see as the realization of the conceptual distinction that is at the core of the GG (which alone justifies its existence for me - tho a realist, I nonetheless like when reality tends toward matching the ideal form 😁). And don't get me wrong, I'm not suggesting to rewrite or revolutionalize the current plotting of easystats, nor even to relocate any of the existing code, I'm just saying that after several years, the current way how plotting works in modelbased really appears to me as convenient, smooth and neat, and I'd like to eventually be able to try adding visualization of new eaystats objects using this approach (not changing existing ones), hence the need to simply move the main method definition lower. It's still in an R&D phase even for me, but I think it's worth testing out.

Regarding 'see', it's vital to the plots creation regardless of the approach, and also for the global easystats visualization environment through themes, geoms, palettes and assemblage of plots. So that's not changing

bwiernik commented 3 years ago

Sure. We can let others weigh in on their thoughts. In terms of development maintenance, especially for external contributors, it makes most sense for me that the answer to "where is this function related to visualization?" is in the package that ostensibly holds all of the visualization functions for easystats, rather than in another package with fairly distinct stated goals (insight/datawizard).

DominiqueMakowski commented 3 years ago

wait no bu I was just talking about moving the function definition, i.e., these lines:

https://github.com/easystats/modelbased/blob/60ac47bf25fb8eb22e29b0b34038e58cf318938e/R/visualisation_recipe.R#L11-L14

so that we can then define methods wherever (including in see)

bwiernik commented 3 years ago

Oh, got it.

I'm saying I think all of the visualisation_recipe stuff (generics, methods, etc.) makes more sense in see.