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

Small fixes for beebop #217

Merged
merged 11 commits into from
Sep 7, 2022
Merged

Small fixes for beebop #217

merged 11 commits into from
Sep 7, 2022

Conversation

muppi1993
Copy link
Contributor

Using v2.5 with beebop I encountered some issues:

  • data/microreact.pkl was not included in the poppunk installation. Added a line to MANIFEST.in to solve that.
  • running generate_visualisations() with cytoscape=True(which results in tree="none") and rank_fit=None resulted in UnboundLocalError: local variable 'rr_distMat' referenced before assignment
    as none of the conditions to assign rr_distMat would be met in visualise.py. (self would be False, since update_db was set to False in assign mode). Solved this by removing condition and tree != "none" which was introduced recently.

Other issue not solved yet: using the --serial flag in assign mode does only add the query samples to the output (rather than all samples in the database as without that flag). For now I'll leave that turned off, but would be nice to get the full output despite this turned on.

@muppi1993 muppi1993 requested a review from johnlees September 7, 2022 09:21
Copy link
Member

@johnlees johnlees left a comment

Choose a reason for hiding this comment

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

I just checked and the microreact.pkl is already in setup.py so I think this is all fine

@johnlees johnlees merged commit 434408b into master Sep 7, 2022
@johnlees johnlees deleted the bacpop-43 branch September 7, 2022 10:37
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