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

Integrate generate_microreact.py script #44

Closed
johnlees opened this issue Apr 5, 2019 · 9 comments
Closed

Integrate generate_microreact.py script #44

johnlees opened this issue Apr 5, 2019 · 9 comments
Assignees
Labels
enhancement New feature or request package Packaging of code

Comments

@johnlees
Copy link
Member

johnlees commented Apr 5, 2019

The script at scripts/generate_microreact.py should be integrated as an exported command in the main package to avoid duplicating functions.

Additionally, the following should be fixed:

  • Use of rapidnj isn't supported (function definition not included)
  • --output isn't clear if it is a prefix/directory/file, and fails if a folder with that name does not exist
@johnlees johnlees added enhancement New feature or request package Packaging of code labels Apr 5, 2019
@johnlees johnlees self-assigned this Apr 5, 2019
@johnlees
Copy link
Member Author

johnlees commented Apr 5, 2019

The dists.csv file should also be removed

@nickjcroucher
Copy link
Collaborator

Added the "--generate-viz" mode to start to address this with update 873f317. This should allow a subset of data to be extracted and visualised as well. I condensed some of the functions within main.py, looks like this may have caused a problem according to the travis checks.

@johnlees
Copy link
Member Author

johnlees commented May 9, 2019

Great, thanks! Working on sorting out the problems now

@nickjcroucher
Copy link
Collaborator

Two issues cropping up with my code. First, there's a problem with the Cytoscape output file:

Building phylogeny Running t-SNE Writing phandango output Copying microreact treeWriting grapetree output Using microreact treeWriting cytoscape output Traceback (most recent call last): File "./PopPUNK/poppunk-runner.py", line 10, in <module> main() File "/lustre/scratch118/infgen/team284/nc3/transmission/PopPUNK/PopPUNK/PopPUNK/__main__.py", line 466, in main outputsForCytoscape(genomeNetwork, isolateClustering, args.output, args.info_csv) File "/lustre/scratch118/infgen/team284/nc3/transmission/PopPUNK/PopPUNK/PopPUNK/plot.py", line 388, in outputsForCytoscape queryList) File "/lustre/scratch118/infgen/team284/nc3/transmission/PopPUNK/PopPUNK/PopPUNK/plot.py", line 534, in writeClusterCsv pd.DataFrame(data=d).to_csv(outfile, columns = colnames, index = False)
Second, there combined clusters being output to the microreact CSV just read "set()". Which appears problematic.

I'll try to have a look at what's going on here.

@nickjcroucher
Copy link
Collaborator

To complete the error for Cytoscape above, the final line is:

KeyError: "None of [['id', 'combined_Cluster']] are in the [columns]"

@johnlees
Copy link
Member Author

I should update the travis-CI to do more thorough checks soon, as we've had a few regressions now as features have been added.

First thought on the cytoscape error (might be the same in phandango and grapetree outputs) is that it might be the graph passed needing to be a dict with graph['combined'] = <graph> vs just a

@nickjcroucher
Copy link
Collaborator

Microreact, Phandango and Grapetree all completed, albeit with the weird "set()" clustering, it only fell over on the Cytoscape output.

@nickjcroucher
Copy link
Collaborator

Fixed in b07ebc2.

@johnlees
Copy link
Member Author

Ah, didn't realise this was specific to the --generate-viz mode. Obviously I didn't check what the readClusters() function actually did! Surprised it got that far at all. Thanks for fixing!

johnlees added a commit that referenced this issue May 28, 2019
Small changes to fix in b07ebc2: need a dict not dict of sets returned for generate viz (#44).
ccoulombe pushed a commit to ccoulombe/PopPUNK that referenced this issue Oct 28, 2022
Move matrix index functions into one place
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request package Packaging of code
Projects
None yet
Development

No branches or pull requests

2 participants