Mmuphin wrapper #6584

renu-pal · 2024-11-22T14:28:14Z

New to tool wrapping , so need a bit of help in finding what is wrong with this code :)

SaimMomin12 · 2024-11-25T09:45:44Z

tools/mmuphin/.shed.yml

+  long_description: |
+    MMUPHin is a Bioconductor package implementing meta-analysis methods for microbial community profiles. It has interfaces for: a) covariate-controlled batch and study effect adjustment, b) meta-analytic differential abundance testing, and meta-analytic discovery of c) discrete (cluster-based) or d) continuous unsupervised population structure.
+
+Overall, MMUPHin enables the normalization and combination of multiple microbial community studies. It can then help in identifying microbes, genes, or pathways that are differential with respect to combined phenotypes. Finally, it can find clusters or gradients of sample types that reproduce consistently among studies


long_description is redundant here.

long_description was duplicated in earlier code. It is completely removed now in 801cc88

I misunderstood and removed the whole thing but made the correction later: 7fafc59

SaimMomin12

@renu-pal Thanks for the contribution. Some of the comments inline.

Please also reduce the test-data size for this file: tools/mmuphin/test-data/CRC_abd.tsv (Ideally, the file needs to be below 1M)

tools/mmuphin/macros.xml

SaimMomin12 · 2024-11-25T10:00:53Z

tools/mmuphin/mmuphin.xml

+        library(ggplot2)
+        library(readr)  
+
+        source(adjust_batch.R)


You will also need to add adjust_batch.R script to your test-data and source that here

tools/mmuphin/mmuphin.xml

Co-authored-by: Saim Momin <[email protected]>

paulzierep · 2024-11-28T17:24:40Z

Fixed with: renu-pal#4
Main fix is:

the imports were not needed, mmuphin has what we need
parsing the needed column variable via awk as in maaslin2

@SaimMomin12 there is no need to add R files to the wrapper and source them, the package provides this functions

Mmuphin wrapper

fixed tests

paulzierep

@bgruening can we merge it ?

paulzierep · 2025-01-07T14:31:01Z

@renu-pal can you convert this to ready for review, I think we can merge it !

tools/mmuphin/mmuphin.xml

bgruening · 2025-01-07T15:25:40Z

tools/mmuphin/mmuphin.xml

+        #end for
+        #set idx_for_awk = ','.join(idx)
+
+        export covariates=`awk -v OFS=',' -F"\t" 'NR == 1 { print $idx_for_awk}' '$input_metadata'` &&


Why are you not doing this in R directly? I think passing the variables via ENV variables is not very clean. I would prefer to pass them to the script via files or even better do the awk filtering directly in R.

tools/mmuphin/mmuphin.xml

bgruening · 2025-01-07T15:43:42Z

tools/mmuphin/mmuphin.xml

+    <command detect_errors="exit_code"><![CDATA[
+
+        ## get the batch name
+        export batch=`awk -v idx='$batch_input' -F"\t" 'NR == 1 { print \$idx }' '$input_metadata'` &&


See my next comment. Can we do this filtering in the R script directly?

Co-authored-by: Björn Grüning <[email protected]>

renu-pal · 2025-01-13T17:02:11Z

Hi @bgruening , I have made the necessary changes, let me know if this works
c1b167c

tools/mmuphin/macros.xml

tools/mmuphin/mmuphin.xml

bgruening · 2025-01-13T20:26:14Z

tools/mmuphin/mmuphin.xml

+
+            <!-- <output name="output">
+                <assert_contents>
+                    <has_size value="150053" delta="1000" />
+                </assert_contents>
+            </output> -->


Should this be removed?

Yes, it was to be removed later but I missed it. Removed it now. Thanks for pointing out :)

@bgruening anything else you want me to look into?

bgruening · 2025-01-14T07:39:06Z

tools/mmuphin/mmuphin.xml

+            <param argument="pseudo_count" type="float" optional="true" label="Pseudo_count" help="Pseudo count to add feature_abd before the methods' log transformation.Default to NULL, in which case will be set to half of minimal non-zero values in feature_abd"/>
+            <param argument="conv" type="float" value="0.0001" optional="true" label="Convergence threshold" help="Convergence threshold for the method's iterative algorithm for shrinking batch effect parameters"/>
+            <param argument="maxit" type="float" value="1000" optional="true" label="Maximum number of iterations" help="Maximum number of iterations allowed for the method's iterative algorithm. Default to 1000"/>
+            <param argument="verbose" type="boolean" truevalue="verbose TRUE" falsevalue="verbose FALSE" checked="true" label="Print verbose information"/> 


we usually don't expose those parameters to the user

@bgruening ,so should I remove them ?

yes and set a useful default

Hi @bgruening , I have the made required changes . Does this work ?
19a33c0

bgruening · 2025-01-14T07:40:37Z

tools/mmuphin/mmuphin.xml

+    <inputs>
+        <param name="input_data" type="data" format="tabular" label="Data (or features) file"/>
+        <param name="input_metadata" type="data" format="tabular" label="Metadata file"/>
+        <param argument="batch_input" type="data_column" data_ref="input_metadata" use_header_names="true"  label="batch" />


Can you please improve all labels and help text. They are not very user-friendly IMHO.

How does a metadata file needs to look like? Or the feature file? "batch"? Maybe "the column in which the batch identifier is species"?

@bgruening does this work?
5109992

bgruening · 2025-01-14T07:40:59Z

tools/mmuphin/mmuphin.xml

+
+
+


can you please clean those things up? thanks

tools/mmuphin/mmuphin.xml

Co-authored-by: Björn Grüning <[email protected]>

tools/mmuphin/mmuphin.xml

Co-authored-by: Björn Grüning <[email protected]>

paulzierep

Please add the option for no covariates

paulzierep · 2025-01-16T08:14:56Z

tools/mmuphin/mmuphin.xml

+        <param argument="covariates_input" type="data_column" data_ref="input_metadata" use_header_names="true" multiple="true" optional="true" label="Covariates Column(s)" help="Select the column(s) in your metadata file that contain covariates (e.g., age, gender, or environmental factors) to be included in the batch correction process."/>
+        <section name="additional_options" title="Additional Options" expanded="true">
+            <param argument="zero_inflation" type="boolean" truevalue="zero_inflation TRUE" falsevalue="zero_inflation FALSE" checked="true" label="Run zero-inflated model"/>            
+            <param argument="pseudo_count" type="hidden" value="NULL"  label="Pseudo_count" help="Pseudo count to add feature_abd before the methods' log transformation.Default to NULL, in which case will be set to half of minimal non-zero values in feature_abd" />


Hi @renu-pal; I would not hide expert parameters, but put them in an extra block as you did ! What Björn meant is to not show to the user is the verbose part. So technical parts hide, expert parts allow.

paulzierep · 2025-01-16T08:16:55Z

tools/mmuphin/mmuphin.xml

+        </configfiles>
+
+    <inputs>
+        <param name="input_data" type="data" format="tabular" label="Abundance (or features) file" help="TSV file where columns represent samples and rows include abundance values for a microbial taxa across those samples"/>


I would be more generic here, this tool is not limited to microbial taxa, it can also be genes, SNPs, path ways...

tools/mmuphin/mmuphin.xml

paulzierep · 2025-01-16T08:50:50Z

tools/mmuphin/mmuphin.xml

+
+        batch <- colnames(meta_data[$batch_input-1])
+        cat("Batch Name:", batch, "\n")
+        covariates_val <- list($covariates_input)


Covariates can be empty (only correct batch). To handel the python None in R, this should work:

# Ensure covariates_input is checked for NULL if (!exists("$covariates_input") || is.null($covariates_input)) { covariates_val <- list() # Assign an empty list if input is NULL or does not exist } else { covariates_val <- list($covariates_input) } covariates <- c() # Process covariates only if they exist if (length(covariates_val) > 0) { for (i in covariates_val) { covariates <- c(covariates, colnames(meta_data[i - 1])) } cat("Covariates Names:", covariates, "\n") } else { cat("No covariates provided.\n") }

@paulzierep this code does not seem to work for some reason when I set covariates input to null. I keep on getting the error : Object 'None' not found.
PS: I removed --- !exists("$covariates_input") from if since it was executing if condition even when covariate was not empty. I believe it is used to just test whether covariate_input exists in the current env , which kept turning false.

i have pushed the other updates you asked for below, and working on how to fix this null issue.
a139a38

paulzierep · 2025-01-16T08:51:37Z

tools/mmuphin/mmuphin.xml

+   </outputs>
+
+    <tests>
+        <test expect_num_outputs="2">


Can you add a test without covariates

Co-authored-by: paulzierep <[email protected]>

renu-pal force-pushed the mmuphin_wrapper branch from 41ef84e to 52c3ce8 Compare November 22, 2024 21:00

renu-pal and others added 2 commits November 22, 2024 22:13

mmuphin wrapper draft with errors

298cadd

Update .shed.yml

ef26199

renu-pal force-pushed the mmuphin_wrapper branch from 52c3ce8 to ef26199 Compare November 22, 2024 21:15

SaimMomin12 reviewed Nov 25, 2024

View reviewed changes

renu-pal and others added 14 commits November 25, 2024 19:18

removing long description from .shed.yml

801cc88

Update tools/mmuphin/macros.xml

f0ca96f

Co-authored-by: Saim Momin <[email protected]>

Update tools/mmuphin/macros.xml

c19da4d

Co-authored-by: Saim Momin <[email protected]>

Update tools/mmuphin/macros.xml

52076ec

Co-authored-by: Saim Momin <[email protected]>

Update tools/mmuphin/macros.xml

bab0879

Co-authored-by: Saim Momin <[email protected]>

Update tools/mmuphin/macros.xml

7c09803

Co-authored-by: Saim Momin <[email protected]>

Update tools/mmuphin/mmuphin.xml

f340c26

Co-authored-by: Saim Momin <[email protected]>

reducing CRC_abd file size and adding adjust_batch.R file

4ee8d65

adding long description into .shed.yml due to linting issue

e861c0f

Update .shed.yml

f65885e

Update .shed.yml

7fafc59

update

f79c79f

update

1b335f1

rm unneeded requs

d86e487

renu-pal and others added 7 commits November 29, 2024 19:03

Merge pull request #4 from paulzierep/mmuphin_wrapper

47d426d

Mmuphin wrapper

changing batch value in test, as first column header is null

f815e86

removing control_output from test

f36aa66

reducing file size

090b476

Update mmuphin.xml

c5af559

fixed tests

e60c159

Merge pull request #5 from paulzierep/mmuphin_wrapper

9c404da

fixed tests

paulzierep approved these changes Jan 7, 2025

View reviewed changes

renu-pal marked this pull request as ready for review January 7, 2025 14:35

SaimMomin12 approved these changes Jan 7, 2025

View reviewed changes

bgruening reviewed Jan 7, 2025

View reviewed changes

renu-pal and others added 2 commits January 12, 2025 22:10

Update tools/mmuphin/mmuphin.xml

7c2d2d7

Co-authored-by: Björn Grüning <[email protected]>

getting column names in R directly

c1b167c

bgruening reviewed Jan 13, 2025

View reviewed changes

bgruening and others added 2 commits January 13, 2025 21:30

Apply suggestions from code review

550cd60

removed unnecessary commented code

8d38c99

bgruening reviewed Jan 14, 2025

View reviewed changes

renu-pal and others added 2 commits January 14, 2025 15:38

Update tools/mmuphin/mmuphin.xml

b6e9c39

Co-authored-by: Björn Grüning <[email protected]>

improving help section

5109992

bgruening reviewed Jan 14, 2025

View reviewed changes

tools/mmuphin/mmuphin.xml Outdated Show resolved Hide resolved

renu-pal and others added 2 commits January 14, 2025 22:34

Update tools/mmuphin/mmuphin.xml

0c902c3

Co-authored-by: Björn Grüning <[email protected]>

removing additional options

19a33c0

paulzierep reviewed Jan 16, 2025

View reviewed changes

renu-pal and others added 2 commits January 16, 2025 14:21

Update tools/mmuphin/mmuphin.xml

663287a

Co-authored-by: paulzierep <[email protected]>

adding test with covariate=null and few other updates

a139a38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mmuphin wrapper #6584

Mmuphin wrapper #6584

renu-pal commented Nov 22, 2024

SaimMomin12 Nov 25, 2024

renu-pal Nov 25, 2024

SaimMomin12 Nov 26, 2024

renu-pal Nov 26, 2024

SaimMomin12 left a comment •

edited

Loading

SaimMomin12 Nov 25, 2024

paulzierep commented Nov 28, 2024 •

edited

Loading

paulzierep left a comment

paulzierep commented Jan 7, 2025

bgruening Jan 7, 2025

bgruening Jan 7, 2025

renu-pal commented Jan 13, 2025 •

edited

Loading

bgruening Jan 13, 2025

renu-pal Jan 13, 2025

renu-pal Jan 13, 2025

bgruening Jan 14, 2025

renu-pal Jan 14, 2025

bgruening Jan 14, 2025

renu-pal Jan 15, 2025

bgruening Jan 14, 2025

renu-pal Jan 14, 2025

bgruening Jan 14, 2025

paulzierep left a comment

paulzierep Jan 16, 2025

paulzierep Jan 16, 2025

paulzierep Jan 16, 2025

renu-pal Jan 16, 2025 •

edited

Loading

renu-pal Jan 16, 2025

paulzierep Jan 16, 2025

Mmuphin wrapper #6584

Are you sure you want to change the base?

Mmuphin wrapper #6584

Conversation

renu-pal commented Nov 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SaimMomin12 left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulzierep commented Nov 28, 2024 • edited Loading

paulzierep left a comment

Choose a reason for hiding this comment

paulzierep commented Jan 7, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

renu-pal commented Jan 13, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

paulzierep left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

renu-pal Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SaimMomin12 left a comment •

edited

Loading

paulzierep commented Nov 28, 2024 •

edited

Loading

renu-pal commented Jan 13, 2025 •

edited

Loading

renu-pal Jan 16, 2025 •

edited

Loading