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

Black configuration is not used #73

Closed
kpj opened this issue Sep 24, 2020 · 3 comments · Fixed by #76
Closed

Black configuration is not used #73

kpj opened this issue Sep 24, 2020 · 3 comments · Fixed by #76
Assignees

Comments

@kpj
Copy link
Contributor

kpj commented Sep 24, 2020

Consider the following Snakefile:

rule all:
    input:
        foo='bar'

And the following configuration file:

[tool.black]
skip-string-normalization = 1

Then execute snakefmt --compact-diff --config config.toml Snakefile.

I would expect the file to pass formatting without any modifications.
However, it says:

=====> Diff for Snakefile <=====

--- original
+++ new
@@ -1,3 +1,3 @@
 rule all:
     input:
-        foo='bar'
+        foo="bar",

[INFO] All done 🎉

It seems like black does not know about the configuration parameters.

Compare this to executing black --diff --config config.toml test.py where test.py only contains 'fubar'.
The output is (as expected) All done! ✨ 🍰 ✨.

@mbhall88
Copy link
Member

Hmm, that's odd. I'll take a look next week.

Notes for future self:

@mbhall88 mbhall88 self-assigned this Sep 25, 2020
mbhall88 added a commit to mbhall88/snakefmt that referenced this issue Oct 1, 2020
@mbhall88
Copy link
Member

mbhall88 commented Oct 1, 2020

Ok @kpj I have a fix (I think) in mbhall88@9f3abcf

One thing for future reference, it is better to use booleans true rather than 1 in TOML. I found in one version of black that it actually failed if 1 was used.

There were two issues causing this bug:

  1. We weren't passing the config past the CLI parsing if it didn't contain [tool.snakefmt]
  2. When creating the black.FileMode object, it actually uses string_normalization, which is the inverse of the CLI arg --skip-string-normalization (so annoying). So I have added in some stuff to handle both flavours

Feel free to try out and see if you have any problems with it.

@kpj
Copy link
Contributor Author

kpj commented Oct 1, 2020

Awesome, thanks for taking a look!

One thing for future reference, it is better to use booleans true rather than 1 in TOML. I found in one version of black that it actually failed if 1 was used.

Makes sense, will do. We should also amend this in snakefmt's README.

Feel free to try out and see if you have any problems with it.

If I try the above example again, I get the output [INFO] All done 🎉
It seems to work nicely :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants