Testing Framework #3
Conversation
There was a problem hiding this comment.
Looks like it's headed in the right direction! I left a few high-level comments within.
I would recommend not committing the output files to the repository. To test, we probably want to compare what the tools output at that instant instead of whenever the outputs were previously generated and checked into the repo. So leaving them out (and possibly even adding them to .gitignore) would be a good way to ensure that every test gets "fresh" results to compare.
instructions.txt
Outdated
| Sorry that the testing process isn't super automated right now! | ||
| This will be fixed very soon in the near future. For now, here's | ||
| how to do the differential testing: |
There was a problem hiding this comment.
Thanks for writing up the instructions! Go right ahead and put this in the main README.md along with the rest of the instructions for stuff to do with the repository.
Makefile
Outdated
| TEST_FILE := DRB1-3123.gfa | ||
| GFA_URL := https://raw.githubusercontent.com/pangenome/odgi/ebc493f2622f49f1e67c63c1935d68967cd16d85/test | ||
|
|
||
| .PHONY: fetch | ||
| .PHONY: fetch test |
There was a problem hiding this comment.
Just stating the obvious: looks like these Makefile changes aren't used for now.
depth.py
Outdated
| with open('python_output.txt', 'w') as f: | ||
| depth_items = depth_map.items() | ||
| for pair in depth_items: | ||
| f.write(f'{pair[0]} {pair[1]}\n') |
There was a problem hiding this comment.
I think just printing this this to stdout would be simpler. If you just do print(f'...') instead of opening a text file as output, it remains possible to both see the output in your console or write it to a file. That is, you can do this:
$ python3 depth.py something.gfa
to see the output with your eyes, or use shell redirection:
$ python3 depth.py > output.txt
to write it to output.txt for testing/comparison purposes. Leaving the output file non-hardcoded will make it easier to manipulate the output when necessary.
There was a problem hiding this comment.
Looking good! Added a few comments—one high-level suggestion is to make the various command-line tools read from stdin and write to stdout, which will avoid hard-coding specific filenames to read/write. (This will make the tools easier for humans to use and also make them more flexible as we build more tooling around them.)
| @@ -8,13 +8,16 @@ def depth(filename): | |||
|
|
|||
| for path in g.paths: | |||
| for segment in path.segment_names: | |||
| name = segment.name | |||
| name = int(segment.name) | |||
There was a problem hiding this comment.
Do we know that segment names are always numbers? (If not, then this call to int(...) might crash.)
| with open('python_output.txt', 'w') as f: | ||
| for pair in sorted_depth_items: | ||
| f.write(f'{pair[0]} {pair[1]}\n') |
There was a problem hiding this comment.
I would recommend just printing this to standard output: i.e., just use print, not f.write (and no need to hard-code an output filename). This will make it easier to use the various scripts for different purposes in the future.
| python3 process.py temp_depth.txt | ||
|
|
||
| python3 depth.py $OPTARG |
There was a problem hiding this comment.
If you take my suggestion above to avoid hard-coding the output filename, you can do > python_output.txt here.
| @@ -0,0 +1,11 @@ | |||
| import sys | |||
There was a problem hiding this comment.
This file could use some comments describing what it does.
| with open('odgi_output.txt', 'w') as f2: | ||
| for i in range(1, len(data)): | ||
| f2.write(f'{data[i][0].strip()} {data[i][1].strip()}\n') |
There was a problem hiding this comment.
I'd also avoid hard-coding the output filename here too. Just using a normal print will make this print to the standard output stream, making things more flexible.
| @@ -0,0 +1,11 @@ | |||
| import sys | |||
|
|
|||
| with open(sys.argv[1], 'r') as f: | |||
There was a problem hiding this comment.
This isn't critical, but for bonus points, you can avoid needing to take a file name as input by just reading from standard input… that is, read directly from sys.stdin. Then you would invoke this script like:
python3 process.py < input.txt > output.txt
to read data from input.txt and write it to output.txt.
Made some modifications to depth.py and added a program to format output of odgi depth