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

Fun with imports :) #8408

Closed
vgoklani opened this issue Nov 1, 2023 · 6 comments
Closed

Fun with imports :) #8408

vgoklani opened this issue Nov 1, 2023 · 6 comments
Labels
question Asking for support or clarification

Comments

@vgoklani
Copy link

vgoklani commented Nov 1, 2023

Hey there,

Thanks for updating Ruff to support the python-black formatter!

I'm using this on Sublime Text 4 via LSP-ruff, and these are my settings:

{
  "log_debug": true,
  "lsp_format_on_save": true,
  "lsp_code_actions_on_save": {
    "lsp_format_on_save": true,
    "source.lsp_format_on_save": true,
    "source.fixAll": true,
    "source.organizeImports": true,
    "source.applyFormat": true
  },
  "clients": {
    "ruff-lsp": {
      "command": [
        "ruff-lsp"
      ],
      "enabled": false,
      "selector": "source.python",
      "initializationOptions": {
        "settings": {
          "args": [],
        },
      },
    },
  },
}

There are a few quirks that I just wanted to point out:

  1. imports that aren't yet used automatically get removed when I save the file.

As I'm writing code, I add import statements, and instinctively hit the save button, only to find that those imports are then removed. I do like how you organize the imports, but I don't like how they disappear! Is it possible to add an additional parameter to separate these two cases.

  1. When writing f-strings, this case is not being caught by Ruff:

    my_string = f"Hello {data["name"]}"

previously python-black would catch the fact that we are using " in both the string declaration and the data dictionary. Ruff is not catching these cases, which leads to run-time errors.

Thanks for releasing Ruff, looking fowrard to more features :)

@zanieb
Copy link
Member

zanieb commented Nov 1, 2023

Thanks for the issue!

  1. You should disable F401 when using your IDE i.e. by adding --ignore F401 to the additional lint command arguments.

  2. That's valid syntax in Python 3.12 now (https://docs.python.org/3/whatsnew/3.12.html#whatsnew312-pep701) which we have full support for. If your target Python version is lower, we can probably detect that and include a violation cc @dhruvmanila

@zanieb zanieb added the question Asking for support or clarification label Nov 1, 2023
@dhruvmanila
Copy link
Member

previously python-black would catch the fact that we are using " in both the string declaration and the data dictionary. Ruff is not catching these cases, which leads to run-time errors.

Can you expand on how is python-black catching this case? Is it throwing an error stating that this syntax isn't supported? I think #6591 would solve your use-case.

@dhruvmanila
Copy link
Member

dhruvmanila commented Nov 2, 2023

For (1), do you wish to just disable the fix and allow the editor to highlight unused imports? If so, you can use the unfixable setting to do that.

For additional context, the organize imports action isn't responsible for removing unused imports. It's the F401 rule whose auto-fix removes them and as @zanieb suggested, you could either disable the rule entirely or just disable the fix.

I hope this helps :)

@charliermarsh
Copy link
Member

Going to close for now, but happy to answer any follow-up questions.

@vgoklani
Copy link
Author

vgoklani commented Nov 7, 2023

@dhruvmanila sorry I missed your reply

I used python-black in sublime text 4. For case #2, it would highlight the row when either ' or " were used incorrectly as part of the f-string.

@dhruvmanila
Copy link
Member

I used python-black in sublime text 4. For case #2, it would highlight the row when either ' or " were used incorrectly as part of the f-string.

By incorrectly, do you mean that the outer and inner quotes were the same? The linked issue in my comment should solve this although how do we go about implementing it isn't final yet.

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

No branches or pull requests

4 participants