jasp-stats / jasp-issues

This repository is solely meant for reporting of bugs, feature requests and other issues in JASP.
58 stars 29 forks source link

Rename BF10 when comparing to best model? #129

Open quentingronau opened 6 years ago

quentingronau commented 6 years ago

Personally, I find it a bit confusing that when the option "Compare to best model" is selected, the output table says "BF10" which is in fact not the Bayes factor against the null model but against the best model. Maybe we could change this in the tables/interface to something more informative?

juliuspfadt commented 1 year ago

@EJWagenmakers, any thoughts?

EJWagenmakers commented 1 year ago

In that case we could replace the "0" by a "b" in the subscripts. However this may create confusion as well. What do you think, @JohnnyDoorn @vandenman

JohnnyDoorn commented 1 year ago

This is indeed a confusing thing! My suggestion: replace 1 and 0 with R and B (or 0, if comparing to null) and add a footnote to explain the subscript: image So here we would have BF_{R, B} as the column header, and add a footnote along the lines of

BF{R, B} denotes the model comparison between the model in each row (R), and the best model (B).

and if comparing to null model

BF{R, 0} denotes the model comparison between the model in each row (R), and the null model (0).

@vandenman do you agree? Then I can update the code (also for Bayesian regression?).

vandenman commented 1 year ago

I agree with the change, and in fact wanted to implement this sometime in the past. It's a bit more difficult to implement than you'd think though. The main issue is that we use Unicode for subscript 0 and 1 but unfortunately the subscript b does not exist in Unicode... (see https://en.wikipedia.org/wiki/Unicode_subscripts_and_superscripts#Latin,_Greek_and_Cyrillic_tables). Instead you have to somehow use html tags but those are (intentionally) removed in jaspTables so this will require some more work.

@JorisGoosen do you remember what needs to be done to allow for HTML tags in jaspTables?

JorisGoosen commented 1 year ago

We could simply add a workaround for the HTML removal part to not remove those subscripts?

JorisGoosen commented 1 year ago

Only thing necessary is changing escapeHtmlStuff to:

    inline static std::string escapeHtmlStuff(std::string input)
    {
        input = replaceBy(input, "&",           "&");
        input = replaceBy(input, "<",           "&lt;");
        input = replaceBy(input, ">",           "&gt;");
        input = replaceBy(input, "&lt;sub&gt",  "<sub>");
        input = replaceBy(input, "&lt;/sub&gt", "</sub>");

        return input;
    }
Kucharssim commented 1 year ago

Another alternative would be to implement LaTeX expressions in our output. That would be nice and enable much more cool functionality, but since my light testing of KaTeX (https://github.com/jasp-stats/INTERNAL-jasp/issues/395) I did not have any time to push this forward.

vandenman commented 1 year ago

Another alternative would be to implement LaTeX expressions in our output.

That would be much more complicated, although it would allow for some nicer output. I think for the short-term we could create a function like asHTML("text") which sets an attribute on the string that tells jaspTable it should not run escapeHtmlStuff on it. That way we can also reuse it when setting data, e.g.,

tb <- createJaspTable()
tb$addColumnInfo(name = "data", title = asHTML("BF<sub><B1/sub>"))
tb[["data"]] = asHTML(paste0("data<sub>", letters, "</sub>"))
which would then show up like so BFB1
dataa
datab
...
datay
dataz

alternatively, we could add two arguments to $addColumnInfo like escapeHTMLinTitle and escapeHTMLinData.

@Kucharssim any preferences for either approach? perhaps adding two more arguments to $addColumnInfo is more discoverable?

Kucharssim commented 1 year ago

That would be much more complicated

Well it doesn't have to be that difficult, see https://github.com/jasp-stats/jasp-desktop/pull/5073. Considering @shun2wang picked this up and it looks like we could potentially have LaTeX support already for the next release, I would prefer to handle it that way than adding short-term solutions, otherwise we'll end up having to support both approaches. But I don't mind too much if we go this way - it obviously depends on how pressing this issue is.

shun2wang commented 1 year ago

Well, I'm not sure if that will go into 0.17.2, some testing is needed there. if we want to have a short-term solution that would probably be a low risky and it's easy to clean it up after 0.17.2 be released?

I don't have any particular preference for this Katex.js but it's an adjunct for me to import formula for JASP note, in fact it's easy to use with jaspHtml too😀

you can comment on this PR https://github.com/jasp-stats/jasp-desktop/pull/5073 and I will have a look if something need to be done.

Kucharssim commented 1 year ago

Well, I'm not sure if that will go into 0.17.2

definitely not in 0.17.2, I meant the one after that

JorisGoosen commented 1 year ago

The fix I mention in https://github.com/jasp-stats/jasp-issues/issues/129#issuecomment-1508261611 should be real easy, low errorchance and should allow for <sub> and </sub> in jaspHtml/jaspTable/etc without it getting messed up.

I can do this in like 5 minutes right now for 0.17.2. And I can also add <sup> and </sup> Then for 0.17.3 or 0.18 we have katex support as well, but these fixes should only have as a sideeffect that people who have <sup> etc in their data might get some missing output?

shun2wang commented 1 year ago

In katex I seted only render a formula with$$formula$$or \\begin formula \\end as elements to be render which will including into a span class name of katex katex-mathml katex-html things, so I am not sure if <sup> will be missing Maybe I misunderstood the meaning?

JorisGoosen commented 1 year ago

The <sup> thing is specifically for the quick fix I suggest in https://github.com/jasp-stats/jasp-issues/issues/129#issuecomment-1508261611

Im just assuming if subscript is wanted superscript is too?

Not related to katex

JorisGoosen commented 1 year ago

Ill just make a branch of this

JorisGoosen commented 1 year ago
image
JorisGoosen commented 1 year ago
image
JorisGoosen commented 1 year ago

It should also work in everything else btw

tomtomme commented 9 months ago

@Kucharssim @JohnnyDoorn So it seems @JorisGoosen merged the patch needed to convert from BF10 to BFRB (see https://github.com/jasp-stats/jasp-issues/issues/129#issuecomment-1508213296)

But the tables in ANOVA and Regression still need to be updated to use that code and show BFRB. Tested with current jasp 0.19 beta

tomtomme commented 5 months ago

still valid with current 0.19beta