therefore hardcoding [-3] (counting from the back) brings us directly below "samples" i.e. where all the directories with sample names are. I guess we might want to change that to get sample names from the design table by a function inside the Snakefile in 0.2. But not here - this merge request is designed to address the bug which Anastasia has found, not code style.
Again, this is not an issue of this branch/ merge request - this is already in the master.
But regardless - just above the line you mention there is:
```
fastq_zip_list = glob.glob(
os.path.join(
params.results_dir,
"samples",
"*",
"*_fastqc",
"*_fastqc.zip"))
```
therefore hardcoding [-3] (counting from the back) brings us directly below "samples" i.e. where all the directories with sample names are. I guess we might want to change that to get sample names from the design table by a function inside the Snakefile in `0.2`. But not here - this merge request is designed to address the bug which Anastasia has found, not code style.
Why is this not clean? params.results_dir hold whatever the user provides in the config file. If it starts with / then we know we are dealing with absolute path and we have to append / as a prefix. In all other cases we deal with relative paths and there is no need for that prefix. How would you propose to re-write this?
Why is this not clean? `params.results_dir` hold whatever the user provides in the config file. If it starts with `/` then we know we are dealing with absolute path and we have to append `/` as a prefix. In all other cases we deal with relative paths and there is no need for that prefix. How would you propose to re-write this?
Using bare / should never be used as it's not portable. Better to use os.path.sep which expands to the correct path separator for the current system.
This is overly complicated. There is a method isabs() in the standard os.path module, which is written exactly for the purpose of checking whether a path is absolute.
You may not even need to do this check, and instead simply return an absolute path for your path, regardless of whether it is relative or absolute. Probably the easiest and certainly the most Pythonic way. You can use os.path.abspath() for that.
Agree with Foivos.
1. Using bare `/` should never be used as it's not portable. Better to use `os.path.sep` which expands to the correct path separator for the current system.
2. This is overly complicated. There is a method `isabs()` in the standard `os.path` module, which is written exactly for the purpose of checking whether a path is absolute.
3. You may not even need to do this check, and instead simply return an absolute path for your path, regardless of whether it is relative or absolute. Probably the easiest and certainly the most Pythonic way. You can use `os.path.abspath()` for that.
changed this line in [version 2 of the diff](/zavolan_group/pipelines/rhea/-/merge_requests/56/diffs?diff_id=2814&start_sha=dafbdbc54b90f7c3e5658e6ed310a9bbce6ab565#0456372e65f8de9e1e46bd1bf062a1278d0b02dd_1117_1116)
