Review footprint_extraction v0.1

[renewiegandt]

Line 3, 19, 37, 41, 321, 335, 344: Call peaks? You should rename it to footprints_extraction.

Line 8 - 17: Are these the only uses or are there more?

Line 32 - 41, 62, 66, 88, 96, 249, 280: First letter in a sentence (e.g. in help message) should be uppercase.

Line 57: " We have no possibility to check if the format of the files is right here though".
I would remove this comment.

Line 62, 66, 88, 96, 280: Add an "Error:" or "ERROR:" in front of the message? You can do this by using logger.error()

Line 80: Casting to int is only possible if the String contains numbers.
You should check if both values (bed_line_array[1], bed_line_array[2])
are numbers before casting them and checking if bed_line_array[2] > bed_line_array[1].
I would also remove the '=' if start and end are the same value something would seem not quite right.

Line 104: Why <2?

Line 344: Instead of "call_peaks needed %s minutes to generate the output", I would write something like
"It took footprint_extraction.py %s minutes to generate the output"

Overall:

• not quite sure if this is an issue but I prefer writing the inline documentation before the code line and as a separate line.
  For example:

  scores_in_peak = np.nan_to_num(np.array(list(bw_open.values(chromosom, peak_start, peak_end)))) #save the scores to an array
  

  to

  #save the scores to an array
  scores_in_peak = np.nan_to_num(np.array(list(bw_open.values(chromosom, peak_start, peak_end))))
  

• I have not lookedinto the functionallity of it yet.

[anastasiia]

1. the old name call_peaks is now changed to the footprints_extraction so far I can see.
2. I don't know what you are meaning with only uses or there are more, as these are the libraries my script needs to function well, and I just wrote the comments behind not to forget what are these libraries for.
3. ok
4. ok
5. I prefer to use info for the error messages as well. I had problems with redirecting of errors and info-messages to a file while using parameter --silent (not to print on terminal) and the only way to solve that problem was to use a logger with info messages only. I have written in the info messages "ERROR" now though to make you happy.
6. you have not looked properly, I check first, if these are digits, and only in this case cast it to int. And there is no need to delete =, as it is exactly the point to make sure, everything is right. If the end position in the bed file is the same as the start position, someone who passed me this bed file was completely wrong, so I can't proceed such a bed file. In this line there is everything fine.
7. this is an extra check to be sure the footprints I am working with, have not the length of 1 or 2 bp. This check is not necessary, but I wanted it to be there just to be sure everything is working well.
8. this refers to the first point. The old name of the script is changed to footprints_extraction now.
9. I think it is a good idea if a line is quite long. In other cases such as if and else statement I prefer to have the comment on the exact same line to make the code readable and easy to understand.
10. let me know when you have tested it.

[anastasiia]

#24

[renewiegandt]

I tested the script and the results look pretty good.

I got an error by setting the output parameter to a path:

python footprints_extraction.py --bigwig /mnt/agnerds/masterjlu2018/testdata/example_git/hg38/buenrostro50k_chr1_fp.bw --bed /mnt/agnerds/masterjlu2018/testdata/example_git/hg38/buenrostro50k_chr1_peaks.bed --output_file /mnt/agnerds/Rene.Wiegandt/10_Master/tmp/review_footprint_extraction.bed --window_length 200 --step 100 --percentage 0
2018-12-28 06:11 : call_peaks.py was called using these parameters: {'step': 100, 'percentage': 0, 'bed': '/mnt/agnerds/masterjlu2018/testdata/example_git/hg38/buenrostro50k_chr1_peaks.bed', 'silent': False, 'bigwig': '/mnt/agnerds/masterjlu2018/testdata/example_git/hg38/buenrostro50k_chr1_fp.bw', 'output_file': '/mnt/agnerds/Rene.Wiegandt/10_Master/tmp/review_footprint_extraction.bed', 'window_length': 200}
2018-12-28 06:11 : reading of the bed file
2018-12-28 06:11 : looking for footprints within peaks
Traceback (most recent call last):
  File "footprints_extraction.py", line 351, in <module>
    main()
  File "footprints_extraction.py", line 339, in main
    write_to_bed_file(all_footprints, args.output_file)
  File "footprints_extraction.py", line 289, in write_to_bed_file
    output_file = open(output_file_name, 'w') #open a file to write
FileNotFoundError: [Errno 2] No such file or directory: 'not_sorted_/mnt/agnerds/Rene.Wiegandt/10_Master/tmp/review_footprint_extraction.bed'

I would either check for a path and throw an error or make it possible to set an outputpath with this parameter.

I also get an error while calling -h to get the help message.

python footprints_extraction.py -h
Traceback (most recent call last):
  File "footprints_extraction.py", line 351, in <module>
    main()
  File "footprints_extraction.py", line 314, in main
    args = parse_args()
  File "footprints_extraction.py", line 42, in parse_args
    args = parser.parse_args()
  File "/mnt/workspace1/rene.wiegandt/work/conda/masterenv-a3a53bf93daef74b8782720be458dcfa/lib/python3.5/argparse.py", line 1726, in parse_args
    args, argv = self.parse_known_args(args, namespace)
  File "/mnt/workspace1/rene.wiegandt/work/conda/masterenv-a3a53bf93daef74b8782720be458dcfa/lib/python3.5/argparse.py", line 1758, in parse_known_args
    namespace, args = self._parse_known_args(args, namespace)
  File "/mnt/workspace1/rene.wiegandt/work/conda/masterenv-a3a53bf93daef74b8782720be458dcfa/lib/python3.5/argparse.py", line 1964, in _parse_known_args
    start_index = consume_optional(start_index)
  File "/mnt/workspace1/rene.wiegandt/work/conda/masterenv-a3a53bf93daef74b8782720be458dcfa/lib/python3.5/argparse.py", line 1904, in consume_optional
    take_action(action, args, option_string)
  File "/mnt/workspace1/rene.wiegandt/work/conda/masterenv-a3a53bf93daef74b8782720be458dcfa/lib/python3.5/argparse.py", line 1832, in take_action
    action(self, namespace, argument_values, option_string)
  File "/mnt/workspace1/rene.wiegandt/work/conda/masterenv-a3a53bf93daef74b8782720be458dcfa/lib/python3.5/argparse.py", line 1016, in __call__
    parser.print_help()
  File "/mnt/workspace1/rene.wiegandt/work/conda/masterenv-a3a53bf93daef74b8782720be458dcfa/lib/python3.5/argparse.py", line 2358, in print_help
    self._print_message(self.format_help(), file)
  File "/mnt/workspace1/rene.wiegandt/work/conda/masterenv-a3a53bf93daef74b8782720be458dcfa/lib/python3.5/argparse.py", line 2342, in format_help
    return formatter.format_help()
  File "/mnt/workspace1/rene.wiegandt/work/conda/masterenv-a3a53bf93daef74b8782720be458dcfa/lib/python3.5/argparse.py", line 278, in format_help
    help = self._root_section.format_help()
  File "/mnt/workspace1/rene.wiegandt/work/conda/masterenv-a3a53bf93daef74b8782720be458dcfa/lib/python3.5/argparse.py", line 208, in format_help
    func(*args)
  File "/mnt/workspace1/rene.wiegandt/work/conda/masterenv-a3a53bf93daef74b8782720be458dcfa/lib/python3.5/argparse.py", line 208, in format_help
    func(*args)
  File "/mnt/workspace1/rene.wiegandt/work/conda/masterenv-a3a53bf93daef74b8782720be458dcfa/lib/python3.5/argparse.py", line 515, in _format_action
    help_text = self._expand_help(action)
  File "/mnt/workspace1/rene.wiegandt/work/conda/masterenv-a3a53bf93daef74b8782720be458dcfa/lib/python3.5/argparse.py", line 602, in _expand_help
    return self._get_help_string(action) % params
ValueError: incomplete format

But I'm not sure if its a problem with your script or with my setup. Could you check that on your setup?

[anastasiia]

The bug with the path to the output file is fixed. The help message is also fixed. Please accept my pull request #24

[renewiegandt]

Fixed by #24