Extraneous Lines in samples.txt #13
Merged
Merged
Conversation
RyanBerger98
commented
Apr 2, 2026
RyanBerger98
left a comment
RyanBerger98
left a comment
Collaborator
Author
There was a problem hiding this comment.
@standage this is ready for review!
Comment on lines
+38
to
+40
| if len(added_samples) > 0: | ||
| with open(workdir / "samples.txt", "a") as fh: | ||
| print(*added_samples, sep="\n", file=fh) |
Collaborator
Author
There was a problem hiding this comment.
This is the fix. Only open samples.txt to write sample names if the number of samples copied over is greater than 0.
Comment on lines
+91
to
+101
| def test_duplicate_samples(tmp_path): | ||
| seq_path = files("ezfastq") / "tests" / "data" / "flat" | ||
| arglist = [seq_path, "test1", "test2", "test3", "--workdir", tmp_path] | ||
| cli.main(arglist) | ||
| with open(tmp_path / "samples.txt", "r") as fh: | ||
| num_lines = len(fh.readlines()) | ||
| assert num_lines == 3 | ||
| cli.main(arglist) | ||
| with open(tmp_path / "samples.txt", "r") as fh: | ||
| num_lines = len(fh.readlines()) | ||
| assert num_lines == 3 |
Collaborator
Author
There was a problem hiding this comment.
Regression test. This fails on the main branch but passes here.
RyanBerger98
commented
Apr 2, 2026
RyanBerger98
left a comment
RyanBerger98
left a comment
Collaborator
Author
There was a problem hiding this comment.
@standage now this is ready for review
Comment on lines
+25
to
+27
| if line.strip(): | ||
| old_name, new_name = cls.parse_name(line, sep="\t") | ||
| name_map[old_name] = new_name |
Collaborator
Author
There was a problem hiding this comment.
The fix for #12. Checks that line actually has content after whitespace has been stripped.
Comment on lines
70
to
89
| @@ -86,3 +87,16 @@ def test_fq_command(tmp_path): | |||
| arglist = ["ezfastq", seq_path, "test1", "test2", "test3", "--workdir", tmp_path] | |||
| run(arglist) | |||
| assert len(list((tmp_path / "seq").glob("*_R?.fastq.gz"))) == 6 | |||
Collaborator
Author
There was a problem hiding this comment.
Updated test to ensure that empty lines in input sample file doesn't break ezfastq
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fixes a bug recently encountered.
When running
ezfastqasamples.txtfile will automatically be generated for the user containing the sample names of all the files that have been copied over.ezfastqwill append sample names to this file rather than overwrite it ifezfastqis ran multiple times with the same working directory.ezfastqavoids writing duplicate sample names to thesamples.txtfile. However, ifezfastqis ran multiple times with the same samples, empty lines will be appended to the file.Closes #12