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.

Sign up for GitHub

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

Jump to bottom

refactor: oligomap output conversion script #115

Merged
merged 18 commits into from
Nov 15, 2023
Merged

refactor: oligomap output conversion script #115

merged 18 commits into from
Nov 15, 2023

Conversation

deliaBlue
Copy link
Collaborator

@deliaBlue deliaBlue commented Oct 30, 2023 •
edited
Loading

This PR closes #54 .

The modifications in this PR are:

  • Refactor and rename the script oligomap_output_to_sam_nh_filtered.py
  • Create the unit test for the script and add the necessary files
  • Modify the CI file to include the script in the static code analysis
  • Modify the subworkflow map.smk to accommodate the refactoring
  • Modify the link to the oligomap_output_to_sam_nh_filtered.py in the pipeline_documentation.md

@deliaBlue deliaBlue requested a review from uniqueg October 30, 2023 21:13
@deliaBlue deliaBlue self-assigned this Oct 30, 2023
@deliaBlue deliaBlue linked an issue Oct 30, 2023 that may be closed by this pull request
refactor: refactor and create unit test for oligomapOutputToSam_nhfiltered.py #54
Closed
@deliaBlue
Copy link
Collaborator Author

The static code analysis test fails as the PR #114 is not merged yet. Once it is, and merge into this PR, it will pass.

@deliaBlue
Merge branch 'dev' into 54-refactor-refactor-and-create-unit-test-for…
1871d47 
…-oligomapoutputtosam_nhfilteredpy

Merge dev branch
@uniqueg uniqueg changed the title refactor: refactor and unit test for oligomap output script refactor: oligomap output script Nov 9, 2023
@uniqueg uniqueg changed the title refactor: oligomap output script refactor: oligomap output conversion script Nov 9, 2023
uniqueg
uniqueg requested changes Nov 9, 2023
View reviewed changes
Copy link
Member

@uniqueg uniqueg left a comment

Choose a reason for hiding this comment

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

I'll leave it at this for now. Let's discuss the comments I made so far before I dig deeper into the code/changes.

.github/workflows/tests.yml Show resolved Hide resolved
scripts/oligomap_output_to_sam_nh_filtered.py Show resolved Hide resolved
scripts/oligomap_output_to_sam_nh_filtered.py Outdated Show resolved Hide resolved
scripts/oligomap_output_to_sam_nh_filtered.py Outdated Show resolved Hide resolved
scripts/oligomap_output_to_sam_nh_filtered.py Outdated Show resolved Hide resolved
workflow/rules/map.smk Outdated Show resolved Hide resolved
workflow/rules/map.smk Outdated Show resolved Hide resolved
@deliaBlue deliaBlue requested a review from uniqueg November 14, 2023 20:03
uniqueg
uniqueg previously approved these changes Nov 15, 2023
View reviewed changes
Copy link
Member

@uniqueg uniqueg left a comment

Choose a reason for hiding this comment

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

I think this looks good know (apart from two minor things that don't need re-review - so just do the changes, resolve the conversations and then you can merge)

scripts/oligomap_output_to_sam_nh_filtered.py Outdated Show resolved Hide resolved
scripts/oligomap_output_to_sam_nh_filtered.py Outdated Show resolved Hide resolved
@deliaBlue deliaBlue merged commit 7d70304 into dev Nov 15, 2023
@deliaBlue deliaBlue deleted the 54-refactor-refactor-and-create-unit-test-for-oligomapoutputtosam_nhfilteredpy branch November 15, 2023 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@uniqueg uniqueg uniqueg left review comments

Assignees

@deliaBlue deliaBlue

Labels
refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

refactor: refactor and create unit test for oligomapOutputToSam_nhfiltered.py
2 participants
@deliaBlue @uniqueg