Change of `include:` spacing between versions 0.8.0 and 0.8.1

[corneliusroemer]

Running snakefmt --check . on a modest repo, I noticed the following change in behaviour between 0.8.0 and 0.8.1

This isn't necessarily a bug/regression, but I thought it'd be good to draw attention to it, maybe it could be added as changed behaviour to release notes or such.

Before:

include: "workflow/snakemake_rules/fetch_sequences.smk"

include: "workflow/snakemake_rules/transform.smk"

include: "workflow/snakemake_rules/nextclade.smk"

After:

include: "workflow/snakemake_rules/fetch_sequences.smk"
include: "workflow/snakemake_rules/transform.smk"
include: "workflow/snakemake_rules/nextclade.smk"

I like the more concise output, the forced spaces between includes I always found odd.

[mbhall88]

Yes, the "after" is indeed the correct behaviour. This was a bug/regression in 0.8.0 that I think slipped by because the test that covered this case was actually incorrect itself. I noticed that when making the fixes for 0.8.1 and adjusted it accordingly :)

It is sort of covered by this line in the changelog

collate consecutive directives after if block

if your case isn't after an if block then I guess it's not quite the same. But close enough?

[corneliusroemer]

It may well be after an if block, as in, not inside the if block but after it. Thanks for confirming this is intended.

I hope it's helpful I relay what changes I find after running new versions through snakefmt. Thanks so much for your maintenance work!

[mbhall88]

I hope it's helpful I relay what changes I find after running new versions through snakefmt. Thanks so much for your maintenance work!

It is very helpful. Thanks for all your reports and patience :)

[corneliusroemer]

My pleasure :)

One suggestion: maybe we could collate a bunch of snakemake 0.8.1 compliant Snakefiles from different people/orgs to do CI on - to check what may change? Would be good to cast a wide net to also find some less common usage patterns.

Unit tests are good but it's hard to have every possible pattern unit tested.

[mbhall88]

You mean basically regression tests?

I like the idea. Would you be willing to coordinate the gathering of these files?

[corneliusroemer]

Yes, whatever we may want to call them - just formatting using the CLI, then doing a diff and failing if it changes something.

The biggest work would be to make sure that the test files are continuously adapted (or ignored) to new pushes/releases.

So I'd probably start off with not too big a code base, consider it optional not required to pass for release.

I'll prep it on my fork once I have a bit of time!

[mbhall88]

Sounds great! Thanks very much for organising!

[bricoletc]

I agree, this would be a good addition @corneliusroemer ! I'd actually started a similar process > 1 year ago, but only did it locally - it's a good complement to unit tests