snakemake / snakefmt

The uncompromising Snakemake code formatter
MIT License
147 stars 28 forks source link

Addition of unwanted whitespaces when a function is called within an expression section of an f-string #220

Closed enixmail closed 4 months ago

enixmail commented 6 months ago

When a function is called within an expression section of an f-string, snakefmt will add unwanted whitespaces around the equal (=) sign when assigning a value to a named parameter and will also add a whitespace before a comma separating the parameters in that function call.

Example code:

import json

job_properties = {"resources": {"mem": "1GB"}}

test = f"job_properties: {json.dumps(job_properties, indent=4)}"

running snakefmt --diff will result in the following:

> snakefmt --diff test_add_unwanted_whitespaces.py 
=====> Diff for test_add_unwanted_whitespaces.py <=====

  import json

  job_properties = {"resources": {"mem": "1GB"}}

- test = f"job_properties: {json.dumps(job_properties, indent=4)}"
+ test = f"job_properties: {json.dumps(job_properties , indent = 4)}"
?                                                    +        + +

Note this was with snakefmt 0.10.0 installed through conda, and the version of black that comes with it sees no problem with that file:

> black --diff test_add_unwanted_whitespaces.py
All done! ✨ 🍰 ✨
1 file would be left unchanged.
mbhall88 commented 6 months ago

Thanks for reporting @enixmail. Which python version are you using?

enixmail commented 6 months ago

Python version 3.12.1

mbhall88 commented 6 months ago

Thought that might be the case. We've had some issues trying to deal with how the tokenizer now presents us with f-strings.

I'll try and get around to this soon

mbhall88 commented 5 months ago

Update: I had a go at fixing this when addressing #222 and it turns out to be extremely difficult to fix. Part of what makes it hard is that the spacing around the = is essentially optional, black will allow either approach, so we can't rely on it to fix it for us. We add the spacing based on some explicit rules (i.e. this spacing is desired outside of a function call context), but trying to build in that context while also being inside of an f-string is proving to be difficult as the tokenizer doesn't give us the spaces, so we don't actually no whether the user had spaces or not without looking directly at the string, which creates a whole new level of complexity.

Anyway, that was probably TMI, but it's more for myself or Brice when we revisit this. In the meantime, I'm sorry, but you'll have to live with the spacing unfortunately.

bricoletc commented 4 months ago

I'll have a go at this soon