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

snakefmt issue related to black #118

Closed
fgypas opened this issue Aug 23, 2021 · 11 comments
Closed

snakefmt issue related to black #118

fgypas opened this issue Aug 23, 2021 · 11 comments
Labels
bug Something isn't working

Comments

@fgypas
Copy link
Member

fgypas commented Aug 23, 2021

Hi

I run into another issue (after #112) when running snakefmt (0.4.2). To get the error please try the following:

git clone -b feature/snakefmt https://git.scicore.unibas.ch/zavolan_group/pipelines/zarp.git
cd zarp
# install snakefmt as following:
python3 -m venv venv
. venv/bin/activate
pip install snakefmt==0.4.2
snakefmt --diff worfklow

When I run:
snakefmt --diff workflow/

I get the following error:

snakefmt.exceptions.InvalidPython: Got error:

Cannot parse: 1:116: f(fastqc_pe=expand(os.path.join(config["output_dir"] , "samples" , "{sample}" , "fastqc" , "{mate}") ,sample =[ifor i in pd.unique(samples_table[samples_table["seqmode"] == "pe"].index.values)] ,mate = "fq2" ,))

while formatting code with black.

Do you know why this happens?

Thank you in advance for your help.

@fgypas fgypas added the bug Something isn't working label Aug 23, 2021
@mbhall88
Copy link
Member

mbhall88 commented Aug 30, 2021

sample =[ifor i

is missing a space between the i and for I think?

@SultanOrazbayev
Copy link

SultanOrazbayev commented Sep 14, 2021

Yes, at some point the '\n' is converted into empty string rather than a space, so if there is some multi-line comprehension statement, then snakefmt will raise the error.

Possibly here, but not sure.

This issue is not present in 0.4.0.

@mbhall88
Copy link
Member

@fgypas the feature/snakefmt branch doesn't seem to exist? I cloned the dev branch and snakefmt ran successfully on the workdlow directory...

@SultanOrazbayev if you could provide a minimal example recreating this it would be very helpful thanks.

@fgypas
Copy link
Member Author

fgypas commented Sep 24, 2021

Hi @mbhall88 Sorry we moved the pipeline to github. Here is what I tried to do with snakefmt, version 0.4.3 installed with conda:

git clone -b dev https://github.com/zavolanlab/zarp.git
git checkout eb398b06e68c52009b1fcb0a8903cc395103b795

Then I run twice snakefmt in the workflow directory.

snakefmt workflow
snakefmt workflow

In the first case it formats the code fine:

[INFO] Writing formatted content to workflow/rules/single_end.snakefile.smk
[INFO] Writing formatted content to workflow/Snakefile
[INFO] Writing formatted content to workflow/rules/paired_end.snakefile.smk
[INFO] All done 🎉

In the second case it throws an error:

[INFO] Writing formatted content to workflow/rules/paired_end.snakefile.smk
snakefmt.exceptions.InvalidPython: Got error:

Cannot parse: 1:116: f(fastqc_pe=expand(os.path.join(config["output_dir"] , "samples" , "{sample}" , "fastqc" , "{mate}") ,sample =[ifor i in pd.unique(samples_table[samples_table["seqmode"] == "pe"].index.values)] ,mate = "fq2" ,))

while formatting code with black.

Please let me know if you can also reproduce this.

@mbhall88
Copy link
Member

@fgypas I have a fix on #120 for this, but in the process I discovered another bug (#121) that would impact the example you gave.

For the record, it was only the Snakefile in your repository that was impacted by #120

@fgypas
Copy link
Member Author

fgypas commented Sep 30, 2021

Thanks a lot @mbhall88 for this fix . I will wait then for #121 to be fixed as well.

Regarding #121 I also observed this and I was not sure if it's a bug or I was doing something wrong when using the single ".

In another pipeline I switched to triple " where I don't get this. For example:

rule a:
    shell:
        """
        (kallisto quant \
        --pseudobam \
        {input} > {output}) \
        2> {log.stderr}
        """

@mbhall88
Copy link
Member

mbhall88 commented Oct 1, 2021

Yeah, using single quotes the way you were is a little cumbersome and triple quotes are far more ergonomic, however, syntax-wise they are perfectly valid and we shouldn't have been indenting them as we are.

@Benjamin-Lee
Copy link

I think I may have an example of this error:

rule test:
    input:
        expand("{accession}/blah.fasta", accession=config["accessions"] if type(config["accessions"]) == list else config["accessions"].split(","))
    output:
        "test.fasta",
    shell:
        "cat {input} > {output}"

Running snakefmt on this works and gives me:

rule test:
    input:
        expand(
            "{accession}/blah.fasta",
            accession=config["accessions"]
            if type(config["accessions"]) == list
            else config["accessions"].split(","),
        ),
    output:
        "test.fasta",
    shell:
        "cat {input} > {output}"

However, running snakefmt on that errors with:

snakefmt.exceptions.InvalidPython: Got error:
```
Cannot parse: 1:109: f(expand("{accession}/blah.fasta" ,accession = config["accessions"]if type(config["accessions"]) == listelse config["accessions"].split(",") ,))
```
while formatting code with black.

@mbhall88
Copy link
Member

mbhall88 commented Oct 8, 2021

@Benjamin-Lee could you try out the tip of dev ? That should have a fix for this. I just haven't closed this issue as that hasn't been merged to master yet as we're waiting on a fix in #121 too

@fgypas
Copy link
Member Author

fgypas commented Oct 13, 2021

For me the dev fix works fine. I tested it.
I am just waiting for #121 as well.

@bricoletc
Copy link
Collaborator

#121 is fixed and merged in, closing this!

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

No branches or pull requests

5 participants