Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ggcompare:Implement layer-level P value correction. #143

Open
HMU-WH opened this issue Sep 23, 2024 · 6 comments
Open

ggcompare:Implement layer-level P value correction. #143

HMU-WH opened this issue Sep 23, 2024 · 6 comments

Comments

@HMU-WH
Copy link

HMU-WH commented Sep 23, 2024

I found that ggsignif has never perfectly resolved the issue of P-value adjustment across panels, and ggpubr::stat_compare_means also has this problem. At the same time, both have deficiencies in adapting to faceting.

To address this, I created ggcompare to fill the gaps in P-value adjustment and adaptability to faceting in both. Its primary function is to focus on mean difference testing.

Due to the different operational logic with ggproto objects, I chose not to submit a PR to optimize on the basis of ggsignif, but rather to refactor. However, my main inspiration still comes from ggsignif.

@const-ae
Copy link
Owner

Hi Hao,

that looks very cool. I am a bit confused about the code in your README, where do you define the logic of e.g. StatCompare?

Best,
Constantin

@HMU-WH
Copy link
Author

HMU-WH commented Sep 23, 2024

Hi Hao,

that looks very cool. I am a bit confused about the code in your README, where do you define the logic of e.g. StatCompare?

Best, Constantin

I did not explicitly provide the source code for StatCompare and GeomBracket in ggcompare because, in my view, these are just two ggproto objects. According to my usual coding habits, I load them as LazyData, directly copying them from my personal toolkit. Another reason is that my English proficiency is not high, and my source code contains a large number of non-English comments, so I did not copy the source code of StatCompare over.

You can view the source code of the corresponding components in R using the following method:

StatCompare$compute_layer
StatCompare$compute_panel

@const-ae
Copy link
Owner

Thanks for the explanation.

I did not explicitly provide the source code for StatCompare and GeomBracket in ggcompare because, in my view, these are just two ggproto objects

Well, it is a bit worrying to load some random binary files from the internet, as those could do anything to my computer. I don't want to suggest that you are doing anything problematic; it's just not good practice and will stop some people from installing your package (for more details see rdaradar post by Bob Rudis).

Another reason is that my English proficiency is not high, and my source code contains a large number of non-English comments, so I did not copy the source code of StatCompare over.

I can totally empathize with the feeling that it is scary for other people to look at your code. However, I would like to encourage you to make the code easily viewable before installation. This will help anybody (for example me :) ) who wants to learn how you managed to do the p-value adjustment across facets.

@HMU-WH
Copy link
Author

HMU-WH commented Sep 23, 2024

Thanks for the explanation.

I did not explicitly provide the source code for StatCompare and GeomBracket in ggcompare because, in my view, these are just two ggproto objects

Well, it is a bit worrying to load some random binary files from the internet, as those could do anything to my computer. I don't want to suggest that you are doing anything problematic; it's just not good practice and will stop some people from installing your package (for more details see rdaradar post by Bob Rudis).

Another reason is that my English proficiency is not high, and my source code contains a large number of non-English comments, so I did not copy the source code of StatCompare over.

I can totally empathize with the feeling that it is scary for other people to look at your code. However, I would like to encourage you to make the code easily viewable before installation. This will help anybody (for example me :) ) who wants to learn how you managed to do the p-value adjustment across facets.

Thank you for your suggestion.

Now you can see the specific implementation process here.

@const-ae
Copy link
Owner

That's great :)

And congrats on the nice code. It is quite readable (even without comments) and your approach to do the calculation in compute_layer is a better choice than my way to do it in compute_group!

Two small things I notice skimming your code:

@HMU-WH
Copy link
Author

HMU-WH commented Sep 23, 2024

you should rather use ggplot2:::parse_safe

I did not notice the situation you mentioned in the documentation for getFromNamespace. Using the ":::" form can prevent passing R CMD CHECK, while using getFromNamespace does not have this issue. Moreover, unless the method parse_safe is removed or changed in ggplot2 (which is unlikely), I believe this usage remains safe.

x |> f(_)

Yes, this is a format supported only in newer versions of R, but at least for now, I do not plan to modify it unless there are users who must use an older version of R.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants