Blank lines removed between Python function and special keywords

[enixmail]

Note: this was ran from commit bbceeeb

It appears that blanks line are removed between the end of a Python function and special Snakemake keywords.

Given this Snakefile:

from os.path import exists
from os import mkdir


def foo():
    print("Hello World!")


workdir: "data"


localrules: run_all


onstart:
    if not exists("logs"):
        mkdir("logs")


rule run_all:
    output:
        my_output="output.txt",
    shell:
        "touch {output.my_output}"


onsuccess:
    print("Workflow finished, no error")


onerror:
    print("An error occurred")

snakefmt formats it this way:

from os.path import exists
from os import mkdir


def foo():
    print("Hello World!")
workdir: "data"
localrules:
    run_all,
onstart:
    if not exists("logs"):
        mkdir("logs")


rule run_all:
    output:
        my_output="output.txt",
    shell:
        "touch {output.my_output}"


onsuccess:
    print("Workflow finished, no error")
onerror:
    print("An error occurred")

Shouldn't the 2 blank lines separating the foo() function and the workdir keyword be kept?

Also, should the special handlers onstart, onsuccess and onerror be treated like rules and have 2 blank lines before and after?

[johanneskoester]

Yes, please ensure two blank lines between every toplevel item.

[johanneskoester]

cc @mbhall88 @bricoletc

[bricoletc]

great spot, i'd noticed this too and unhappy with it.

@johanneskoester are you saying there should be two blank lines before workdir but also between workdir and localrules?

[mbhall88]

I disagree that there should be two lines between a python function and workdir. Python functions, by convention, should have one line spacing and workdir feels to be more like a normal variable assignment, so

from os.path import exists
from os import mkdir


def foo():
    print("Hello World!")


workdir: "data"


something else

Should be

from os.path import exists
from os import mkdir


def foo():
    print("Hello World!")

workdir: "data"

something else

[enixmail]

PEP8 states clearly "Surround top-level function and class definitions with two blank lines" and "Method definitions inside a class are surrounded by a single blank line". In the example above we are talking about a top level function, so I believe 2 blank lines are required between the end of the foo() function and the workdir directive.

from os.path import exists
from os import mkdir


def foo():
    print("Hello World!")


workdir: "data"

...

[mbhall88]

Right you are.

[johanneskoester]

Yes, I agree, two blank lines feel right.

[bricoletc]

Great. Thanks a lot for bringing this up @enixmail !
Running from 9919e4b the diff from the snakefile you gave at the top of this issue is:

- localrules: run_all
?            --------
+ localrules:
+     run_all,

So the double spacing you suggested is in place.

I'll keep this open for now as it might fuel more discussion.