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

Bug: Incorrect node text when __slots__ and class variables are used (3.11) #53

Closed
aroberge opened this issue Sep 25, 2022 · 7 comments
Closed

Comments

@aroberge
Copy link

Apologies for not providing a self-contained test with executing for the following, as I don't know how.
The following raises a ValueError exception:

class F:
    __slots__ = ["a", "b"]
    a = 1

With Python 3.10 (and prior), the text of the node identified causing the exception is:

class F:

With Python 3.11, we get this:

class F: __slots__ = ["a", "b"] a = 1

This is not a typo: the text of the node identified a single line, with no \n, which is clearly a SyntaxError.

This is the only bug I found when running the unit tests for friendly_traceback with executing 1.1.0 and Python 3.11. Everything still works for me with Python 3.6 to 3.10, although I found that the tests I run take about 2 to 3 times as long as compared with using executing 0.8.3. The slowdown seemed to be already present with executing 1.0.0.

@15r10nk
Copy link
Collaborator

15r10nk commented Sep 25, 2022

I have currently no idea how the new 3.11 code can cause this result.
It is definitely not rewriting your source code 🙂.

can you provide some more information?

  • The traceback for the ValueError might be useful.
  • how do you get the text? is it done with asttokens?

I have currently also no explanation for the performance degradation. The executing test suite got significant faster. There is maybe something special about the way you use executing. Do you check a lot of instructions which provide invalid results ex.node==None?

@alexmojaki
Copy link
Owner

I'm betting that what's happening is:

  1. executing in 3.11 is able to identify the class definition node, while pre 3.11 it cannot
  2. friendly-traceback itself is collapsing the multiple lines of source code into one for display
  3. Before 3.11, friendly-traceback simply picks the line class F: from the traceback since there is no node from executing, so it doesn't have multiple lines to collapse together.

@alexmojaki
Copy link
Owner

Also there were versions 0.9.0 and 0.9.1 between 0.8.3 and 1.0.0, can you confirm the exact version where the slowdown happens?

@aroberge
Copy link
Author

Using Python 3.10, average of 3 runs of my unit tests, rounded to the nearest second

  • executing 1.1.0: 19s
  • executing 1.0.0: 19s
  • executing 0.9.1: 8s
  • executing 0.9.0: 8s

About your previous comment, from what I can tell:

  1. Executing is indeed able to identify the class definition node, while pre 3.11 cannot
  2. friendly-traceback is NOT collapsing the multiples lines (see below)
  3. Before 3.11, I don't think that friendly_traceback picked up the line from the traceback - unless that was stack_data's doing.

After https://github.com/friendly-traceback/friendly-traceback/blob/b284e47eb12778f5bb68c1fe366802df8a5ea750/friendly_traceback/tb_data.py#L267, inserting the line:

print(f"{node_info=}")

for the class slot example yields

node_info=(None, (0, 8), 'class F:')

when using Python 3.10, and

node_info=(<ast.ClassDef object at 0x00000213EBD7E200>, None, 'class F: __slots__ = ["a", "b"] a = 1')

when using Python 3.11

@alexmojaki
Copy link
Owner

  1. friendly-traceback is NOT collapsing the multiples lines (see below)

Isn't it doing that here?

https://github.com/friendly-traceback/friendly-traceback/blob/b284e47eb12778f5bb68c1fe366802df8a5ea750/friendly_traceback/frame_info.py#L238

Before 3.11, I don't think that friendly_traceback picked up the line from the traceback - unless that was stack_data's doing.

https://github.com/friendly-traceback/friendly-traceback/blob/b284e47eb12778f5bb68c1fe366802df8a5ea750/friendly_traceback/frame_info.py#L220

@aroberge
Copy link
Author

Dang, my bad about the collapse of the multiple lines. I believe that this was needed before, but perhaps it is no longer the case with the new executing. I'll test this right away.

As for the slowdown, I did indicate that it occurs when going from 0.9.1 to 1.0.0

@alexmojaki
Copy link
Owner

Dang, my bad about the collapse of the multiple lines. I believe that this was needed before, but perhaps it is no longer the case with the new executing. I'll test this right away.

Whether or not it was 'needed' is unrelated to executing and entirely about how you choose to display things in friendly. I don't think I understand why you chose that collapsing in the first place.

What's changed now is that identified nodes can be compound statements like class definitions. Multiline nodes were possible before, but they were expressions formatted that way which were still valid after collapsing.

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

No branches or pull requests

3 participants