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

WPS237 Found a too complex f string - this may be too strict #1921

Closed
Andrew-Sheridan opened this issue Mar 2, 2021 · 3 comments · Fixed by #2070
Closed

WPS237 Found a too complex f string - this may be too strict #1921

Andrew-Sheridan opened this issue Mar 2, 2021 · 3 comments · Fixed by #2070
Labels
bug Something isn't working

Comments

@Andrew-Sheridan
Copy link

What's wrong

The following line raises WPS237

self.info(f"Count={count:,}")
  37:19    WPS237 Found a too complex `f` string
  self.info(f"Count={count:,}")

How it should be

It should not do that, as that does not seem an overly complex f-string.

It is formatting an int as a comma separated value: f"count={count:,}" -> "count=1,234"

  • this is the same as "count={count:,}".format(count=count)

Flake8 version and plugins

{
  "dependencies": [],
  "platform": {
    "python_implementation": "CPython",
    "python_version": "3.7.3",
    "system": "Darwin"
  },
  "plugins": [
    {
      "is_local": false,
      "plugin": "flake8-bandit",
      "version": "2.1.2"
    },
    {
      "is_local": false,
      "plugin": "flake8-broken-line",
      "version": "0.3.0"
    },
    {
      "is_local": false,
      "plugin": "flake8-bugbear",
      "version": "20.11.1"
    },
    {
      "is_local": false,
      "plugin": "flake8-comprehensions",
      "version": "3.3.1"
    },
    {
      "is_local": false,
      "plugin": "flake8-darglint",
      "version": "1.6.0"
    },
    {
      "is_local": false,
      "plugin": "flake8-debugger",
      "version": "4.0.0"
    },
    {
      "is_local": false,
      "plugin": "flake8-docstrings",
      "version": "1.5.0, pydocstyle: 5.1.1"
    },
    {
      "is_local": false,
      "plugin": "flake8-eradicate",
      "version": "1.0.0"
    },
    {
      "is_local": false,
      "plugin": "flake8-string-format",
      "version": "0.3.0"
    },
    {
      "is_local": false,
      "plugin": "flake8_commas",
      "version": "2.0.0"
    },
    {
      "is_local": false,
      "plugin": "flake8_isort",
      "version": "4.0.0"
    },
    {
      "is_local": false,
      "plugin": "flake8_quotes",
      "version": "3.2.0"
    },
    {
      "is_local": false,
      "plugin": "mccabe",
      "version": "0.6.1"
    },
    {
      "is_local": false,
      "plugin": "naming",
      "version": "0.11.1"
    },
    {
      "is_local": false,
      "plugin": "pycodestyle",
      "version": "2.6.0"
    },
    {
      "is_local": false,
      "plugin": "pyflakes",
      "version": "2.2.0"
    },
    {
      "is_local": false,
      "plugin": "rst-docstrings",
      "version": "0.0.14"
    },
    {
      "is_local": false,
      "plugin": "wemake_python_styleguide",
      "version": "0.15.2"
    }
  ],
  "version": "3.8.4"
}

pip information

pip 21.0.1

cannot show pip freeze

OS information

macOS Catalina 10.15.7 (19H114

@Andrew-Sheridan Andrew-Sheridan added the bug Something isn't working label Mar 2, 2021
@sobolevn sobolevn added this to the Version 0.15.x milestone Mar 2, 2021
@vnmabus
Copy link

vnmabus commented Mar 14, 2021

I also have a problem with the strictness of this rule regarding len. Somehow f"The length is {len(l)}." is considered a too complex f-string.

@sobolevn
Copy link
Member

These two are different:

  • Original problem by @Andrew-Sheridan is about formatting, it should be allowed
  • Problem @vnmabus is about putting logic into f strings, it won't be ever allowed. We even made an exceptional rule for people who like to use them (still wps does not recommend it), but prefer to keep them as simple as referencing variables. So, that's the only use-case we support

I am going to fix the first one in 0.15.3, I am working on it right now. 👋

@sobolevn
Copy link
Member

sobolevn commented Jun 21, 2021

Ok, I was wrong 🙂 Sorry, I am not using f string, so I forgot about it.

We do allow some simple expressions: and yes, len(a) is not allowed because of this code:

# Function call with empty arguments is okay
        elif isinstance(format_value, ast.Call) and not format_value.args:
            return True

sobolevn added a commit that referenced this issue Jun 21, 2021
sobolevn added a commit that referenced this issue Jun 21, 2021
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

Successfully merging a pull request may close this issue.

3 participants