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

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

Closed
enixmail opened this issue Feb 21, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@enixmail
Copy link

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
Copy link
Member

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

@enixmail
Copy link
Author

Python version 3.12.1

@mbhall88
Copy link
Member

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
Copy link
Member

mbhall88 commented Apr 10, 2024

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
Copy link
Collaborator

I'll have a go at this soon

bricoletc added a commit to bricoletc/snakefmt that referenced this issue May 6, 2024
bricoletc added a commit to bricoletc/snakefmt that referenced this issue May 7, 2024
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

3 participants