Giter Site home page Giter Site logo

Comments (20)

bneradt avatar bneradt commented on May 27, 2024 2

I don't mean to come across as alarmist, but this is a serious bug. The https://github.com/apache/trafficserver project relies upon autopep8 to format its Python files, and this single issue keeping us from moving to fedora:39 because its system version of Python is 3.12. We use a pre-commit hook to block any commits that fail autopep8, so Apache Traffic Server devs currently cannot upgrade to fedora:39. Can the maintainer (@hhatto) please comment on the status of this issue and provide an ETA for the landing of its fix?

Thank you for this tool. It has been helpful to the Apache Traffic Server project.

Kind regards,
Brian

from autopep8.

TheKangaroo avatar TheKangaroo commented on May 27, 2024 2

I see the same problem with the newline after { in f strings but only with the pre-commit v2.0.4 hook.
If run autopep8 v2.0.4 from the CLI everything seems to be okay.
I have the following line in my code:

url=f'https://gitlab.example.example.example.com/api/v4/projects/{project_id}'

With the autopep8 CLI (just the whitespaces will be fixed, e.g. what I would expect):

url = f'https://gitlab.example.example.example.com/api/v4/projects/{project_id}'

With autopep8 pre-commit (the linebreak after { will be inserted):

url = f'https://gitlab.example.example.example.com/api/v4/projects/{
    project_id}'

My pre-commit config is:

  - repo: https://github.com/hhatto/autopep8
    rev: v2.0.4
    hooks:
      - id: autopep8

from autopep8.

bneradt avatar bneradt commented on May 27, 2024 1

@hhatto: Thanks for the reply.

Just in case it's helpful, autopep8 on Python 3.12 greatly changes the white space within format strings, not only the addition of newlines. It adds or subtracts a variety of types of white space in various places. Here's some samples of a run of it on the apache/trafficserver code base:

Removing white space

diff --git a/tests/gold_tests/bad_http_fmt/bad_http_fmt.test.py b/tests/gold_tests/bad_http_fmt/bad_http_fmt.test.py
index 8e84e18e1..059971175 100644
--- a/tests/gold_tests/bad_http_fmt/bad_http_fmt.test.py
+++ b/tests/gold_tests/bad_http_fmt/bad_http_fmt.test.py
@@ -45,7 +45,7 @@ ts.Disk.ip_allow_yaml.AddLines([
     '    action: allow',
     '    methods:',
     '      - GET',
-    f'      - {random_method}',
+    f' - {random_method}',
 ])

Adding spaces around colons:

diff --git a/tests/gold_tests/autest-site/trafficserver.test.ext b/tests/gold_tests/autest-site/trafficserver.test.ext
index 1ebf39e0d..59de90f49 100755
--- a/tests/gold_tests/autest-site/trafficserver.test.ext
+++ b/tests/gold_tests/autest-site/trafficserver.test.ext
@@ -409,9 +409,9 @@ def MakeATSProcess(obj, name, command='traffic_server', select_ports=True,
             port_str += " {ssl_port}:quic {ssl_portv6}:quic:ipv6".format(
                 ssl_port=p.Variables.ssl_port, ssl_portv6=p.Variables.ssl_portv6)
         if enable_proxy_protocol:
-            port_str += f" {p.Variables.proxy_protocol_port}:pp {p.Variables.proxy_protocol_portv6}:pp:ipv6"
+            port_str += f" {p.Variables.proxy_protocol_port}: pp {p.Variables.proxy_protocol_portv6}: pp: ipv6"
             if enable_tls:
-                port_str += f" {p.Variables.proxy_protocol_ssl_port}:pp:ssl {p.Variables.proxy_protocol_ssl_portv6}:pp:ssl:ipv6"
+                port_str += f" {p.Variables.proxy_protocol_ssl_port}: pp: ssl {p.Variables.proxy_protocol_ssl_portv6}: pp: ssl: ipv6"
         #p.Env['PROXY_CONFIG_HTTP_SERVER_PORTS'] = port_str
         p.Disk.records_config.update({
             'proxy.config.http.server_ports': port_str,

Addition of newlines

You mentioned this above:

@@ -73,8 +74,9 @@ tr = Test.AddTestRun()
 tr.Processes.Default.StartBefore(ts)
 tr.Processes.Default.Command = (
     server_cmd(1) +
-    fr" ; printf 'GET {random_path}HTTP/1.0\r\n" +
-    fr"Host: localhost:{Test.Variables.server_port}\r\n" +
+    fr"
+printf 'GET {random_path}HTTP/1.0\r\n" +
+    fr"Host: localhost: {Test.Variables.server_port}\r\n" +
     r"X-Req-Id: 0\r\n\r\n'" +
     f" | nc localhost {ts.Variables.port} >> client.log" +
     " ; echo '======' >> client.log"

Here's a particularly extreme example:

diff --git a/tests/gold_tests/cache/background_fill.test.py b/tests/gold_tests/cache/background_fill.test.py
index 56125a492..c0d02fbb4 100644
--- a/tests/gold_tests/cache/background_fill.test.py
+++ b/tests/gold_tests/cache/background_fill.test.py
@@ -58,7 +58,7 @@ class BackgroundFillTest:
                 "dest_ip=* ssl_cert_name=server.pem ssl_key_name=server.key")
 
             self.ts[name].Disk.records_config.update({
-                "proxy.config.http.server_ports": f"{self.ts[name].Variables.port} {self.ts[name].Variables.ssl_port}:ssl",
+                "proxy.config.http.server_ports": f"{self.ts[name].Variables.port} {self.ts[name].Variables.ssl_port}: ssl",
                 "proxy.config.http.background_fill_active_timeout": "0",
                 "proxy.config.http.background_fill_completed_threshold": "0.0",
                 "proxy.config.http.cache.required_headers": 0,  # Force cache
@@ -111,15 +111,41 @@ class BackgroundFillTest:
         tr = Test.AddTestRun()
         self.__checkProcessBefore(tr)
         tr.Processes.Default.Command = f"""
-curl -X PURGE --http1.1 -vs http://127.0.0.1:{self.ts['for_httpbin'].Variables.port}/drip?duration=4;
-timeout 2 curl --http1.1 -vs http://127.0.0.1:{self.ts['for_httpbin'].Variables.port}/drip?duration=4;
+curl -X PURGE --http1.1 -vs http: //127.0.0.1: {self.ts['for_httpbin'].Variables.port}/drip?duration=4;
+timeout 2 curl --http1.1 -vs http://127.0.0.1:{self.ts['for_httpbin         '                                                        ]
+        .
+                                               V
+                                               a
+                                               r
+                                               i
+                                               a
+                                               b
+                                               l
+                                               e
+                                               s
+                                               .
+                                               p
+                                               o
+                                               r
+                                               t
+                                               } /
+        d
+        r
+        i
+        p
+        ?
+        d
+        u
+        r
+        a
+        t
+        i
+        o
+        n
+        =
+        4
 sleep 4;

Removing newlines

In addition to additional newlines, it seems to also sometimes remove newlines:

-curl --http1.1 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?duration=4
-"""
-        tr.Processes.Default.ReturnCode = 0
-        tr.Processes.Default.Streams.stderr = "gold/background_fill_1_stderr.gold"
-        self.__checkProcessAfter(tr)
-
+curl --http1.1 -vsk https://127.0.0.1:{self.ts['for_httpbin'].Variables.ssl_port}/drip?d uration=4 """         tr.Processes.Default.ReturnCode = 0         tr.Processes.Default.Streams.stderr = "gold/background_fill_1_stderr.gold"         self.__checkProcessAfter(tr) 

Adding trailing white space

Note that the previous patch actually adds a trailing space at the end, which may not render in github.

from autopep8.

hruskape avatar hruskape commented on May 27, 2024 1

I have been hitting similar problem like @mvo5 . I have had identical version in use autopep8 2.0.4 (pycodestyle: 2.10.0). For me solution was to upgrade pycodestyle to 2.11.1 which is bringing python3.12 support [1].

[1] https://github.com/PyCQA/pycodestyle/blob/main/CHANGES.txt

from autopep8.

jngrad avatar jngrad commented on May 27, 2024 1

The autopep8 v2.1.0 bugfix doesn't seem cover a few edge cases with pycodestyle 2.11.0 and 2.11.1:

  • high indentation levels with short strings:
    $ code_sample="if 1:\n    if 1:\n        if 1:\n            err_msg = rf'provided index [{str(n)[1:-1]}] is out of range for shape [{str(list(shape))[1:-1]}]'"
    $ echo -e "${code_sample}"
    if 1:
        if 1:
            if 1:
                err_msg = rf'provided index [{str(n)[1:-1]}] is out of range for shape [{str(list(shape))[1:-1]}]'
    $ echo -e "${code_sample}" | autopep8 -
    if 1:
        if 1:
            if 1:
                err_msg = rf'provided index [{
                    str(n)[1:-1]}] is out of range for shape [{str(list(shape))[1:-1]}]'
  • low indentation levels with long strings:
    $ code_sample="if 1:\n    err_msg = f'please provide a test name as argument, like {prefix}lb_cpu__p3m_cpu (got {sys.argv})'"
    $ echo -e "${code_sample}"
    if 1:
        err_msg = f'please provide a test name as argument, like {prefix}lb_cpu__p3m_cpu (got {sys.argv})'
    $ echo -e "${code_sample}" | autopep8 -
    if 1:
        err_msg = f'please provide a test name as argument, like {
            prefix}lb_cpu__p3m_cpu (got {sys.argv})'
  • running with the --aggressive mode:
    $ code_sample="class A:\n    def __str__(self):\n        return f'{self.__class__.__name__}({self.selected_mode.__class__.__name__})'"
    $ echo -e "${code_sample}"
    class A:
        def __str__(self):
            return f'{self.__class__.__name__}({self.selected_mode.__class__.__name__})'
    $ echo -e "${code_sample}" | autopep8 -
    class A:
        def __str__(self):
            return f'{self.__class__.__name__}({self.selected_mode.__class__.__name__})'
    $ echo -e "${code_sample}" | autopep8 --aggressive -
    class A:
        def __str__(self):
            return f'{self.__class__.__name__}({
                self.selected_mode.__class__.__name__})'

Environment:

$ python -V
Python 3.12.3
$ autopep8 --version
  autopep8 2.1.0 (pycodestyle: 2.11.1)
$ cat /etc/lsb-release
DISTRIB_ID=Ubuntu
DISTRIB_RELEASE=24.04
DISTRIB_CODENAME=noble
DISTRIB_DESCRIPTION="Ubuntu Noble Numbat (development branch)"

I can drop the --aggressive flag, since there is a proposal to remove it. Yet the other edge cases remain problematic. While Python 3.12 allows line breaks in f-strings (3.12 release notes, section PEP 701: Syntactic formalization of f-strings, paragraph Multi-line expressions and comments), my project supports multiple Python releases, and Python 3.10 and 3.11 raise SyntaxError: unterminated string literal. As a workaround, one can add # nopep8 at the end of impacted f-strings to avoid formatting by autopep8.

from autopep8.

razvan avatar razvan commented on May 27, 2024

It even corrupts sources by adding newlines to fstrings, as can be tested with: https://github.com/stackabletech/image-tools/pull/6/files#diff-cdc0846192c47ce22d8f9a51ee3c0ed9847a222827f12d00a89569b815d92e57R177

from autopep8.

supakeen avatar supakeen commented on May 27, 2024

I'm seeing the same behavior where it's corrupting f-strings by adding newlines inside replacements. Using autopep8 2.0.4 and Python 3.12.0.

autopep8: commands[0]> bash -c 'python -m autopep8 --diff --max-line-length 120 -a -a -a -j0 -r --exit-code osbuild/ assemblers/* devices/* inputs/* mounts/* runners/* sources/* stages/*.* stages/test/*.py tools/'
--- original/stages/org.osbuild.debug-shell
+++ fixed/stages/org.osbuild.debug-shell
@@ -31,7 +31,8 @@

     unit = f"""
 [Unit]
-Description=Early root shell on {tty} FOR DEBUGGING ONLY
+Description=Early root shell on {
+        tty} FOR DEBUGGING ONLY
 DefaultDependencies=no
 IgnoreOnIsolate=yes
 ConditionPathExists={tty}

from autopep8.

hhatto avatar hhatto commented on May 27, 2024

There are two issues to be discussed and they are listed in order.

1. First Issue

First, @amirstm 's example is incorrect.

$ echo connector = f"socks5://{user}:{password}:{url}:{port}"
connector = fsocks5://{user}:{password}:{url}:{port}
$ echo connector = f"socks5://{user}:{password}:{url}:{port}" | autopep8 -
connector = fsocks5: // {user}: {password}: {url}: {port}

The echo command is not outputting the correct Python f-string example because you did not specify a single quote as an argument.
I recognize the following as correct examples In this case, there is no particular problem with the operation of autopep8.

$ echo 'connector = f"socks5://{user}:{password}:{url}:{port}"'
connector = f"socks5://{user}:{password}:{url}:{port}"
$ echo 'connector = f"socks5://{user}:{password}:{url}:{port}"' | autopep8 -
connector = f"socks5://{user}:{password}:{url}:{port}"

environment:

$ python -V
Python 3.12.0
$ autopep8 --version
autopep8 2.0.4 (pycodestyle: 2.11.1)

2. Second Issue

@razvan @supakeen @bneradt Thanks for reporting.

#712 (comment)
#712 (comment)
#712 (comment)

Second, a line break is inserted at the point after the { in the above f-string where the variable is specified.
This is a surprise to autopep8 users.
We believe this behavior needs to be improved. One idea is to have f-string not change any line breaks, etc.

If you have an opinion on this behavior, please let us know.

Thanks

from autopep8.

supakeen avatar supakeen commented on May 27, 2024

@hhatto correct, I personally feel like autopep8 should likely leave any indentation (including newlines) in multi-line strings alone.

There might be some discussion to be had about things that are expected to be formatted inside { and } in f-strings, but adding a newline isn't :)

from autopep8.

miguelmartins3 avatar miguelmartins3 commented on May 27, 2024

Similar issue in this snippet of code:
image
Hope it helps to troubleshoot or testing.

from autopep8.

zcutlip avatar zcutlip commented on May 27, 2024

I see the same problem with the newline after { in f strings but only with the pre-commit v2.0.4 hook. If run autopep8 v2.0.4 from the CLI everything seems to be okay. I have the following line in my code:

I'm wondering, is it possible you're running the CLI within a virtualenv with a different version of python?

If I run it from the CLI with my default python, 3.12, it still breaks my f-strings. If I have autopep8 installed in a project's virtualenv with python 3.11 there's no problem

from autopep8.

zcutlip avatar zcutlip commented on May 27, 2024

Edit: of course this only applies to Mac, and in particular homebrew python, but hopefully it's helpful

If you came here like me in hopes of a workaround, this seems to be working for me:

check if 'brew' exists and if the default python3 version is 3.12, and if so, add python 3.11 to start of $PATH

if _command_exists "brew";
then
    # HACK: autopep8 is currently broken on python 3.12
    # temporarily set python 3.11 at start of PATH
    # https://github.com/hhatto/autopep8/issues/712
    if python3 --version | grep -E '^Python 3\.12' >/dev/null;
    then
        PATH="$(brew --prefix [email protected])"/libexec/bin:$PATH
        export PATH
    fi
fi

from autopep8.

mvo5 avatar mvo5 commented on May 27, 2024

One interesting find is that it seems to be virtualenv inside py312 that is causing the issues. I was trying to write a test case for the (excellent btw) tests that autopep8 has and struggled and then noticed:

$ .tox/autopep8/bin/python --version
Python 3.12.2
$ .tox/autopep8/bin/python -m autopep8 --version
autopep8 2.0.4 (pycodestyle: 2.10.0)
$ .tox/autopep8/bin/python -m autopep8 --diff -r ./test/
/usr/lib/python3.12/site-packages/autopep8.py:182: DeprecationWarning: lib2to3 package is deprecated and may not be able to parse Python 3.10+
  from lib2to3.pgen2 import tokenize as lib2to3_tokenize
...
--- original/./test/mod/test_util_checksum.py
+++ fixed/./test/mod/test_util_checksum.py
@@ -30,5 +30,5 @@
     tempfile.flush()
 
     digest = TEST_RESULT[algorithm]
-    full_digest = f"{algorithm}:{digest}"
+    full_digest = f"{algorithm}: {digest}"
     assert checksum.verify_file(tempfile.name, full_digest), "checksums mismatch"

which looks clearly incorrect. However with the non-virtualenv version:

$ python3 --version
Python 3.12.2
$ python3 -m autopep8 --version
autopep8 2.0.4 (pycodestyle: 2.10.0)
/usr/lib/python3.12/site-packages/autopep8.py:182: DeprecationWarning: lib2to3 package is deprecated and may not be able to parse Python 3.10+
  from lib2to3.pgen2 import tokenize as lib2to3_tokenize
$ 

so here no diff is shown which is expected.

from autopep8.

TheKangaroo avatar TheKangaroo commented on May 27, 2024

I'm wondering, is it possible you're running the CLI within a virtualenv with a different version of python?
If I run it from the CLI with my default python, 3.12, it still breaks my f-strings. If I have autopep8 installed in a project's virtualenv with python 3.11 there's no problem

I just checked an my autopep8 installation is via brew, which wraps autopep8 in python 3.11, so no wonder it works as expected:

cat /opt/homebrew/bin/autopep8
───────┬──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
       │ File: /opt/homebrew/bin/autopep8
───────┼──────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
   1   │ #!/opt/homebrew/opt/[email protected]/bin/python3.11
   2   │ # -*- coding: utf-8 -*-
   3   │ import re
   4   │ import sys
   5   │ from autopep8 import main
   6   │ if __name__ == '__main__':
   7   │     sys.argv[0] = re.sub(r'(-script\.pyw|\.exe)?$', '', sys.argv[0])

For the breaking part, I think pre-commit always runs python in a virtual env, so yes, this matches the observed behaviour of problems with python 3.12 in a virtual env.

from autopep8.

supakeen avatar supakeen commented on May 27, 2024

@mvo5 That's a very interesting observation especially as you have the same versions of Python installed in the virtual environment and outside of it. Could you show the output of pipdeptree (you'll have to install it, the package is called pipdeptree as well) in both the virtual environment and outside of it for autopep8?

from autopep8.

mvo5 avatar mvo5 commented on May 27, 2024

AFAICT the output inside the virtualenv and outside are identical (diff shows no difference). It an interesting issue, I really wonder how the fact that it runs inside/outside a virtualenv can have this effect (and why only on py312).

from autopep8.

mvo5 avatar mvo5 commented on May 27, 2024

I have been hitting similar problem like @mvo5 . I have had identical version in use autopep8 2.0.4 (pycodestyle: 2.10.0). For me solution was to upgrade pycodestyle to 2.11.1 which is bringing python3.12 support [1].

[1] https://github.com/PyCQA/pycodestyle/blob/main/CHANGES.txt

Thanks, that is amazing. I think that unblocks us now.

from autopep8.

zcutlip avatar zcutlip commented on May 27, 2024

will there be a new release of autopep8 in order to trigger an update of pycodstyle?

from autopep8.

hhatto avatar hhatto commented on May 27, 2024

autopep8 v2.1.0 has been released.
version 2.1.0 now supports pycodestyle v2.11.0 and above.

from autopep8.

zcutlip avatar zcutlip commented on May 27, 2024

Can confirm. Several format strings in pyonepassword are getting wrapped per PEP 701. This breaks python 3.9-3.11 compatibility.

I've added # nopep8, but would be nice if it was possible to disable these fixes in a config, or to specify a minimum python version compatibility mode

from autopep8.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.