All in title, f"{x=}" implemented in https://bugs.python.org/issue36817
are not liked by unparse:  https://github.com/python/cpython/pull/13473,

I don’t think this is worth changing. I’m planning on removing the expr_text field before beta 1. At which point I don’t think there will be any work to do here. 

See https://mail.python.org/pipermail/python-dev/2019-May/157493.html

Also, can you explain what’s currently broken? I’ll need to see some example code that doesn’t work in order to understand your change and if the ast change will affect it.

Never mind on the failing code. I see your test cases now.

Just for clarification for future readers;

I landed a PR that used f"{x=}" in some new functionalities, and that broke the buildbots, the buildbots do test that many of the `Lib/` files can be round-tripped ast->unparse->ast, and as ast-unparse did not understood f-debug, if roundtripped f"{x=}" to f"{x!r}". The PRs CIs did not fail as apparently they don't test this functionality.

So currently not having support for f-string-debug in ast-unparse kind of prevent using f-string debug format in the stdlib.

I'm happy if `expr_text` get removed, and having unparse round-trip to `x={x!r}` sounds sufficient to me; it may make like of formatters like black a bit harder maybe ? Though I believe black is likely not using ast but lib2to3.

Thanks for the recap, Matthias. Once I remove expr_text (hopefully this weekend!), we should still at least add tests to make sure nothing fails, even with the ast-expanded version of the = f-strings.

Notice that test_tools will fail if  f'{x=}' becomes f'x={x!r}' when unparsed as it compares the text of the file and the text of the roundtrip of the ast of the file

I thought it was comparing the AST of the file to the AST of the unparsed
AST.

So it's actually checking if parse(unparse(x)) is indempotent and not
wether parse(unparse(x)) is indempotent.

So x={x!r} should be fine.

On Wed, May 22, 2019, 09:22 Pablo Galindo Salgado <report@bugs.python.org>
wrote:

>
> Pablo Galindo Salgado <pablogsal@gmail.com> added the comment:
>
> Notice that test_tools will fail if  f'{x=}' becomes f'x={x!r}' when
> unparsed as it compares the text of the file and the text of the roundtrip
> of the ast of the file
>
> ----------
> nosy: +pablogsal
>
> _______________________________________
> Python tracker <report@bugs.python.org>
> <https://bugs.python.org/issue37003>
> _______________________________________
>

Actually, it checks if the dump is the same:

 class ASTTestCase(unittest.TestCase):
    def assertASTEqual(self, ast1, ast2):
        self.assertEqual(ast.dump(ast1), ast.dump(ast2))

    def check_roundtrip(self, code1, filename="internal"):
        ast1 = compile(code1, filename, "exec", ast.PyCF_ONLY_AST)
        unparse_buffer = io.StringIO()
        unparse.Unparser(ast1, unparse_buffer)
        code2 = unparse_buffer.getvalue()
        ast2 = compile(code2, filename, "exec", ast.PyCF_ONLY_AST)
        self.assertASTEqual(ast1, ast2)

> Notice that test_tools will fail if  f'{x=}' becomes f'x={x!r}'

I arrived at the wrong conclusion as Matthias points out.

>>> import ast
>>> ast.dump(compile("f'{x=}'","<string>","exec",ast.PyCF_ONLY_AST))
"Module(body=[Expr(value=JoinedStr(values=[FormattedValue(value=Name(id='x', ctx=Load()), conversion=114, format_spec=None, expr_text='x=')]))], type_ignores=[])"
>>> ast.dump(compile("f'{x!r}'","<string>","exec",ast.PyCF_ONLY_AST))
"Module(body=[Expr(value=JoinedStr(values=[FormattedValue(value=Name(id='x', ctx=Load()), conversion=114, format_spec=None, expr_text=None)]))], type_ignores=[])"

if expr_text is removed those strings will be the same, so we will be ok. Sorry for the confusion.

mbussonn: can you re-check this in light of the changes for issue37050? I'm not sure if there's still a problem that needs fixing or not.

I believe all is fine now. Thanks !