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

Split happy data into INDEL and SNP tables #1241

Merged
merged 12 commits into from
Nov 11, 2020
Merged

Conversation

sophie22
Copy link
Contributor

@sophie22 sophie22 commented Jun 30, 2020

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md has been updated

This is an attempt to resolve issue #1233
Data from the happy file is split into indel_data and snp_data and presented in separate tables in the report to allow for different thresholds to be set during conditional formatting set in the config file. Column headers now have an '_indel' or '_snp' suffix, so that they can be referred to in the config file.
Potential future improvement could include an option set in the config file whether or not to split the happy values.

into indel and snv tables
 as there are now 2 and they need to have separate names
the problem might me that happy only expects to generate one plot (table)
need to add separate sections for the 2 tables
typo!!
create separate sets of headers with '_indel' or '_snp' suffix
@sophie22 sophie22 marked this pull request as ready for review June 30, 2020 20:07
@sophie22
Copy link
Contributor Author

It's not failing the automated tests but it also doesn't show the data in the actual report for the split tables right now

@sophie22
Copy link
Contributor Author

Stuck on trying to figure out what 'headers' mean in the table.plot(data, header, config)
and how to have the data column headers the same (so that data is correctly put into both tables after reading in the file content) but the HTML header tags named differently for indel and snp so that they can be referred to using different col_header names in the config file.

had to change the headers when storing the file data into separate dict so that later on they are different and generate unique HTML tags
might not be the neatest way but it works now and passes the automated tests as well
@sophie22
Copy link
Contributor Author

sophie22 commented Jul 1, 2020

had to change the headers when storing the file data into separate dicts so that later on they are different and generate unique HTML tags (which can be used in the config file for conditional formatting)
might not be the neatest way but it works now and passes the automated tests as well

Copy link
Member

@ewels ewels left a comment

Choose a reason for hiding this comment

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

Looks great! Tested locally and working perfectly 🚀

Thanks for the contribution, and sorry for the slow review / merge.

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.

None yet

2 participants