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

[FEATURE, TEST] Making find_deletions write to an output file and not to std::out. And adding tests. #37

Merged
merged 3 commits into from
Nov 24, 2020

Conversation

simonsasse
Copy link
Member

@simonsasse simonsasse commented Nov 2, 2020

Resolves 1/2 of #21

Copy link
Collaborator

@Irallia Irallia left a comment

Choose a reason for hiding this comment

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

Thanks for your work!
Could you please rebase on the current master? And connect the PR to the related issue (Button at the bottom of this page.). And write at least resolves #ISSUENUMBER in the description of this PR.
Most of my requests are more like discussion points.

src/find_deletions/deletion_finding_and_printing.cpp Outdated Show resolved Hide resolved
src/find_deletions.cpp Outdated Show resolved Hide resolved
Copy link
Collaborator

@Irallia Irallia left a comment

Choose a reason for hiding this comment

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

Wonderful, thank you very much!

Copy link
Member

@smehringer smehringer left a comment

Choose a reason for hiding this comment

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

May I suggest something that does not need to be changed or part of this PR:
You go from functions foo() that print to std::cout to function foo(std::ofstream & out_file) that prints to a file. If you would do

template<typename stream_type>
void foo(stream_type & stream)
{
    stream << // ...
}

You can pass either a file OR std::cout. Writing to an dedicated output file is always very helpful in most application but I find sometimes, when writing pipelines, it can be very handy if the file can also be written to std::cout. This of course depends on your App and whether you think it may or may not be executed as part of a pipeline. If not you can ignore my suggestion, if so you might want to consider writing to cout unless a output_path is given. :) Just ideas

Note that there is also a output_stream seqan3 concept for checking that is has the << operator.

include/find_deletions/deletion_finding_and_printing.hpp Outdated Show resolved Hide resolved
src/find_deletions/deletion_finding_and_printing.cpp Outdated Show resolved Hide resolved
src/find_deletions/deletion_finding_and_printing.cpp Outdated Show resolved Hide resolved
test/CMakeLists.txt 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.

None yet

3 participants