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

First commit for Panaroo tool #1

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

FMG0411
Copy link

@FMG0411 FMG0411 commented Nov 4, 2024

No description provided.

tools/panaroo/.shed.yml Outdated Show resolved Hide resolved
Copy link
Owner

@SaimMomin12 SaimMomin12 left a comment

Choose a reason for hiding this comment

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

Hi @FMG0411

I have added some comments and suugestion in line. Please work on them and update the Pull request. I will review again.

tools/panaroo/.shed.yml Outdated Show resolved Hide resolved
tools/panaroo/.shed.yml Outdated Show resolved Hide resolved
tools/panaroo/test-data/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/test-data/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/test-data/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/test-data/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/test-data/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/test-data/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/test-data/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/test-data/panaroo.xml Outdated Show resolved Hide resolved
Copy link
Owner

@SaimMomin12 SaimMomin12 left a comment

Choose a reason for hiding this comment

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

Hi @FMG0411

I think, it would be a wise decision to reduce the tool arguments and only keep that are essential. Additionally, it would be nice to group them based on sections. Such as (Mode, Matching, Refind, Graph Correction, Gene Alignment)

We also need to extend the test case more.

tools/panaroo/macros.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
Copy link
Owner

@SaimMomin12 SaimMomin12 left a comment

Choose a reason for hiding this comment

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

Overall looks good, please look at the some of the suggestion that I have made in this PR.

Major things:

  • Add missing output files
  • Add parameters for Graph correction and Gene alignment sections

After all the above suggestions, you should have some test passing. If not, we can have a look.

tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Show resolved Hide resolved
Comment on lines +109 to +110
<data format="csv" name="gene_p_a" label="${tool.name} on ${on_string} Gene Presence Absence" from_work_dir="out/gene_presence_absence.csv"/>
<data format="fasta" name="core_gene_aln" label="${tool.name} on ${on_string} Core Gene Alignment" from_work_dir="out/core_gene_alignment.aln"/>
Copy link
Owner

Choose a reason for hiding this comment

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

Please add the following files to output as well:

├── combined_DNA_CDS.fasta
├── combined_protein_cdhit_out.txt
├── combined_protein_cdhit_out.txt.clstr
├── combined_protein_CDS.fasta
├── final_graph.gml
├── gene_data.csv
├── gene_presence_absence.csv
├── gene_presence_absence_roary.csv
├── gene_presence_absence.Rtab
├── pan_genome_reference.fa
├── pre_filt_graph.gml
├── struct_presence_absence.Rtab
└── summary_statistics.txt

For gml, file format you can specify datatype as text for now

Copy link
Author

Choose a reason for hiding this comment

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

I can't find some of these Output files in here: https://gthlab.au/panaroo/#/gettingstarted/output

tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
tools/panaroo/panaroo.xml Outdated Show resolved Hide resolved
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