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

maaslin2 --> maaslin3 #656

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from
Open

maaslin2 --> maaslin3 #656

wants to merge 1 commit into from

Conversation

Daenarys8
Copy link
Contributor

ping #653

Signed-off-by: Daena Rys <[email protected]>
J. McIver},
year = {2020},
@Manual{Mallick2024,
title = {MaAsLin 2: Refining and extending generalized
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This says maaslin 2


```{r run-maaslin2, warning=FALSE, results="hide"}
```{r run-maaslin3, warning=FALSE, results="hide"}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you modify these chunk options, please make them to use quarto syntax. --> that way we gradually adopt the full quarto syntax in the book

Jiaxian and Maharjan, Sagun and Mallick,
Himel and Franzosa, Eric A. and Thompson,
Kelsey N. and Nearing, Jacob T. and Huttenhower, Curtis},
year = {2024},
note = {R/Bioconductor package},
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is not Bioconductor package

@@ -1136,14 +1136,17 @@ @Article{Das2020
microbiomes with bias .pdf}
}

@Manual{Mallick2020,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment on lines +335 to +344
formula = '~ patient_status',
normalization = 'TSS',
transform = 'LOG',
augment = TRUE,
standardize = TRUE,
max_significance = 0.1,
median_comparison_abundance = TRUE,
median_comparison_prevalence = FALSE,
max_pngs = 10,
cores = 1,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use either " or ', not both. I think " is more common in the book so use it

Comment on lines +355 to +360
head(signif_results, 20) %>%
dplyr::mutate_if(is.numeric, .funs = function(x){(
as.character(signif(x, 3)))}) %>%
knitr::kable() %>%
kableExtra::kable_styling("striped", position = 'center') %>%
kableExtra::scroll_box(width = "800px", height = "400px")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is too complex and ordinary reader might not understand what is happening here. I think you could just print the table with kable

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moreover, elsewhere we use functions without defining the package, i.e., knitr::kable() --> kable()

Comment on lines -343 to +344
reference = "patient_status,Control",
normalization = "TSS",
standardize = FALSE,
# filtering was previously performed
min_prevalence = 0)
maaslin3_out <- maaslin3(input_data = tse,
output = "DAA_example",
formula = '~ patient_status',
normalization = 'TSS',
transform = 'LOG',
augment = TRUE,
standardize = TRUE,
max_significance = 0.1,
median_comparison_abundance = TRUE,
median_comparison_prevalence = FALSE,
max_pngs = 10,
cores = 1,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maaslin3 seems to have lots of different options. However, here we just want minimum example.

That would be probably:


maaslin3_out <- maaslin3(
    input_data = tse,
    output = "DAA_example",
    formula = "~ patient_status",
    normalization = "TSS",
    transform = "LOG",
    verbosity = "WARN"
    )

Comment on lines +367 to +368
```{r, summary-plot}
knitr::include_graphics("DAA_example/figures/summary_plot.png")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is ok to plot

@@ -34,7 +34,7 @@ gsEasy
igraph
IntegratedLearner
knitr
Maaslin2
maaslin3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be also added to DESCRIPTION to remotes

Comment on lines +353 to +354
signif_results <- read.csv('DAA_example/significant_results.tsv',
sep='\t')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not look good. The indentation is not good

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this file the only format for results? I tried to quickly check, but it seems that the summary table is only available in the file

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

Successfully merging this pull request may close these issues.

2 participants