-
Notifications
You must be signed in to change notification settings - Fork 171
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
Add TSV file support and a demo #2428
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello there! If that is not quite the suprise 😄
So first of all thanks for your commitment to add a another file format. We are basically in a development freeze in favour of SeqAn3 but appreciate contributions. But I have some open requests for this. The most important thing is that I don't see any tests, which would be required before this can be added. The second thnig is that it feels more as it could be part of a text_io module which is a generic text file not associated with any particular file formats. Another format could be csv for example and this module would then be structured more like the seq_io.
CTestTestfile.cmake | ||
cmake_install.cmake | ||
Makefile | ||
.vscode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That looks like a fixup of in-source builds. Can you please remove this but keep the .vscode
and make it a separate commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that non of these should be changed, not even .vscode
. If you need custom exclusion you can always add those to .git/info/exclude
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
well true, but .vscode
is a common hidden directory for visual studio code which gains popularity. So I wouldn't mind but I have no strong feelings either.
/*! | ||
* @tag FileFormats#Tsv | ||
* @headerfile <seqan/tsv_io.h> | ||
* @brief Variant callinf format file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* @brief Variant callinf format file. | |
* @brief Variant calling format file. |
Why is this bound to variant calling?
As far as I understand it merely reads in tab-separated text files.
Accordingly I would put this all into a text_io module, from which tsv is one supported format. Csv could be another. Similar to seq_io.
/*! | ||
* @class TsvRecord | ||
* @implements FormattedFileHeaderConcept | ||
* @headerfile <seqan/vcf_io.h> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
copy n paste errors
Hey! I needed to modify huge amounts of TSV files and as SeqAn is the one and only library to do this efficiently I wanted to contribute some code 😊