Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

newlines formatting #93

Closed
Smeds opened this issue Jan 19, 2021 · 4 comments
Closed

newlines formatting #93

Smeds opened this issue Jan 19, 2021 · 4 comments
Labels
bug Something isn't working enhancement New feature or request

Comments

@Smeds
Copy link

Smeds commented Jan 19, 2021

I have some questions about the number of new lines required by snakefmt

Example

if config["programs"]["Duplicates"] == "fgbio":

    include: "../rules/Alignment/fgbio.smk"
    print("Test")
    include: "../rules/SNV/freebayes.smk"
else:
    if config["programs"]["markduplicate"] == "GPU":

        include: "../rules/Alignment/GPU_alignment.smk"
    else:

        include: "../rules/Alignment/bwa-mem.smk"
        include: "../rules/Alignment/MarkDuplicates.smk"
include: "../rules/SNV/freebayes.smk"
include: "../rules/SNV/mutect2.smk"

would change to

  if config["programs"]["Duplicates"] == "fgbio":
  
      include: "../rules/Alignment/fgbio.smk"
 
 
 print("Test")
 
      include: "../rules/SNV/freebayes.smk"
 
 
  else:
      if config["programs"]["markduplicate"] == "GPU":
  
          include: "../rules/Alignment/GPU_alignment.smk"
 
 
      else:
  
          include: "../rules/Alignment/bwa-mem.smk"
 
          include: "../rules/Alignment/MarkDuplicates.smk"
  include: "../rules/SNV/freebayes.smk"
  include: "../rules/SNV/mutect2.smk"

How many new lines are required seems to be a bit inconsistent and sometimes a bit excessive, atleast for if/else code blocks:

  1. Why is 2 new lines added after an "include" inside a if/else code block?
  2. Why is the indentation removed for print("Test") and two new lines added?
  3. Why is a new line introduced between the include statements in the last else block but not for the two last lines?
  4. Shouldn't there be new lines after the last "else" code block?
Smeds added a commit to clinical-genomics-uppsala/Twist_DNA that referenced this issue Jan 19, 2021
… are applied are a bit inconsistent right now, but I have submitted an issue for this snakemake/snakefmt#93
@bricoletc bricoletc added the bug Something isn't working label Jan 19, 2021
@bricoletc
Copy link
Collaborator

bricoletc commented Jan 19, 2021

Hi again @Smeds, these are good points, thank you for going into this level of detail.

  1. We decided to separate distinct snakemake keywords by two newlines, but perhaps inside such a block should be one.
  2. Is definitely a bug
  3. This is deliberate (I'll dig into why) but could conceivably be changed
  4. I agree

@bricoletc bricoletc added the enhancement New feature or request label Jan 19, 2021
@bricoletc bricoletc mentioned this issue Jan 19, 2021
bricoletc added a commit that referenced this issue Jan 19, 2021
* Bugfix: properly deal with indentation of python code nested in
snakemake syntax
* Space out keywords when they come out of a nested python code context
* Nested, consecutive, identical keywords now not spaced out
@bricoletc
Copy link
Collaborator

Ok, 6fb3400 implements your points 2, 3, and 4. (digging into 3, can't find a good justification for it).

For point 1, we are actually mirroring black:

if p:

    class p:
        elem1 = 2


else:

    class a:
        pass

And I find this readable so will keep it.

If (for readability) you place your print statement before the includes, it will now look like:

if config["programs"]["Duplicates"] == "fgbio":

    print("Test")

    include: "../rules/Alignment/fgbio.smk"
    include: "../rules/SNV/freebayes.smk"


else:
    if config["programs"]["markduplicate"] == "GPU":

        include: "../rules/Alignment/GPU_alignment.smk"


    else:

        include: "../rules/Alignment/bwa-mem.smk"
        include: "../rules/Alignment/MarkDuplicates.smk"


include: "../rules/SNV/freebayes.smk"
include: "../rules/SNV/mutect2.smk"

What do you think @Smeds ?

@mbhall88
Copy link
Member

Agree with @bricoletc on all accounts

bricoletc added a commit that referenced this issue Jan 20, 2021
Treat special edge case where comments follow snakemake directives
nested in python code.
@Smeds
Copy link
Author

Smeds commented Jan 22, 2021

Looks great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants