easystats / bayestestR

:ghost: Utilities for analyzing Bayesian models and posterior distributions
https://easystats.github.io/bayestestR/
GNU General Public License v3.0
561 stars 55 forks source link

To do for CRAN submission #35

Closed strengejacke closed 5 years ago

strengejacke commented 5 years ago

My impression is that bayestestR is not far away from its initial CRAN submission. Would you agree? Let's put together a to-do list to check what needs to be done before submission:

What else? I also made some suggestions who takes over which tasks, feel free to add / remove yourself from the list. ;-)

DominiqueMakowski commented 5 years ago

I agree ☺️

But yes I feel like we are close to an initial release 🎉

strengejacke commented 5 years ago

For plot(), I thought about a ggplot-solution, similar to what I implemented in sjstats: https://strengejacke.github.io/sjstats/articles/bayesian-statistics.html#test-for-practical-equivalence

We may use this as a temporary solution in _bayestestR:, and move it to report later. Or we can directly start implementing it in report. bayestestR is a package that both is at low- and high-level, so it's difficult to say what will be the best solution.

So you would vote for report (or equivalent, but I think that report can also comprise plot-methods)?

why is it an issue to depend on the github master version instead of the CRAN?

Do you mean to have a separate branch for CRAN submissions? Or am I getting you wrong here? Afaik, the Remotes-field is no valid field for a DESCRIPTION file, so we need to remove it somewhere...

strengejacke commented 5 years ago

Huh, where's my answer?

DominiqueMakowski commented 5 years ago

I think it would be too much for report to also handle the plots. I see report as the high-level textual output creator, but there is room (and need) for a high-level visual output generator.

It's mainly a matter of scope and dependencies, as I would see quite many other functions for which a plot method would be needed (correlation, estimate, even performance with some fancy sankey diagrams for the R2s). In the end, in easystats, ggplot would be used as a dep in only one package, "visualise" or "visualization" or "figure" or "easyviz" or "vizreport" or whatnot, used by people that want to.

Starting from the bottom there would be:

I think it's easier to start with separate functions/features, and merge them if in the future if we see that it makes sense, rather than splitting, IMO harder from a user perspective (introducing breaking changes, features that disapear and so on).

Maybe we could try to implement these plot methods separately first, and then merge them if we feel it's the right way to go for the next release?

Afaik, the Remotes-field is no valid field for a DESCRIPTION file

Hum, devtools checks don't complain about the remote fields for me, I didn't know it was not valid. In that case yes, we need to remove them.

strengejacke commented 5 years ago

I think it's easier to start with separate functions/features, and merge them if in the future if we see that it makes sense, rather than splitting, IMO harder from a user perspective (introducing breaking changes, features that disapear and so on).

Ehm, so what would you suggest? Put plot()-function into the different packages and merge them later into on "vizreport" package? Or the other way round? :-D

CRAN submission When you submit your package to CRAN, all of its dependencies must also be available on CRAN. For this reason, release() will warn you if you try to release a package with a Remotes field.

(https://cran.r-project.org/web/packages/devtools/vignettes/dependencies.html)

DominiqueMakowski commented 5 years ago

If we summarise:

One way to satisfy both could be to have the visualreport package as standalone, but to reexport its methods and functions in others packages where applicable. For instance, bayestestR would reexport the plotting functions for its things, and correlation and estimation also (mutatis mutandis). This way, people would only import bayestestR and be able to use the plotting methods. This separation will mostly be "for us", to have a common repo with common plotting tools for easystats packages.

What do you think about this way of combining separation with unification?

strengejacke commented 5 years ago

What do you think about this way of combining separation with unification?

Especially if we would have own "geoms" or "scales" for our functions, a separate package would make sense, and then the related other packages would reexport the plot-function. This is rather a long-term goal, especially since there's none of the affected package on CRAN yet. ;-)

That said, maybe we should first push the next release of insight, because now there are too many things bayestestR depends on in the forthcoming insight-update. So maybe we can be more relaxed now and plan an initial release of bayestestR somewhen by the end of April / May?

DominiqueMakowski commented 5 years ago

Agreed for pushing insight first, which is the cornerstone. I think that BayestestR is quite close to be initial-release ready, as the two main remaining things are ci() and documentation overhaul.

Of course, we can use the time to polish a bit more everything (and the JOSS paper) and start thinking seriously about visualizeR, for which we can develop the support for bayestestR's equivalence test first, as we have a quite clear view on what and how to implement it :)

DominiqueMakowski commented 5 years ago

Good way of doing the CRAN checks on multiple platforms ourselves:

https://blog.r-hub.io/2019/03/26/why-care/

We should maybe add in a CONTRIBUTING file somewhere (in easystats?) some sort of "checklist" before PR merging, including for example these CRAN checks, but also styler, pkgdown, why not lintr, etc.

strengejacke commented 5 years ago

That's a good idea! For other contributers ("users"), I would say that code with related tests are nice to have for a PR, but checking etc. should be done from our side (or at least: we should not make this a requirement for PRs, because hurdles for external contributions might be too high then).

strengejacke commented 5 years ago

I've updated the website now. I'll look into the docs etc. Is there anything serious missing or not yet working that needs to be addressed before CRAN submission?

strengejacke commented 5 years ago

Just saw, this page did not compile as intended.

DominiqueMakowski commented 5 years ago

image

Haha insight 2 is finally fully built on CRAN. Will go forward with bayestestR

DominiqueMakowski commented 5 years ago

I submitted this version this morning, so will close this :)