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

inspect.getsource (findsource) not working as expected when duplicate class is used #106727

Closed
Viicos opened this issue Jul 13, 2023 · 8 comments
Closed
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@Viicos
Copy link
Contributor

Viicos commented Jul 13, 2023

Bug report

Consider the following example:

import inspect

class Model:
    a = 1

class Model:
    a = 2

print(Model.a)
#> 2
print(inspect.getsource(Model))
#> class Model:
#>     a = 1

I think we should expect the last class to be used (it works as expected if we use functions instead of classes).

This issue was raised in an external project:

And I'm facing this issue while working on:

Let me know if this should be fixed, I'll take a deeper look at what was done in #10307 and see if I can make a PR

Your environment

  • CPython versions tested on: 3.11.4
  • Operating system and architecture: Ubuntu

Linked PRs

@Viicos Viicos added the type-bug An unexpected behavior, bug, or error label Jul 13, 2023
@Viicos Viicos changed the title inspect.getsource not working as expected when duplicate class is used inspect.getsource (findsource) not working as expected when duplicate class is used Jul 13, 2023
@AlexWaygood AlexWaygood added the stdlib Python modules in the Lib dir label Jul 13, 2023
@gaogaotiantian
Copy link
Member

The method has already been changed from regex to ast. I can work on the PR if you are okay with it. If you want to fix this yourself, the code piece is at the similar place, just switched to an ast based searching.

@gaogaotiantian
Copy link
Member

Just BTW, the last class may not be the one you need.

if cond:
    class A:
        ...
else:
    class A:
        ...

We could probably need some heuristics to figure out which class it should be.

@Viicos
Copy link
Contributor Author

Viicos commented Jul 14, 2023

The method has already been changed from regex to ast. I can work on the PR if you are okay with it. If you want to fix this yourself, the code piece is at the similar place, just switched to an ast based searching.

Yes by look at what was done in #10307 I meant improve this ast based search.

We could probably need some heuristics to figure out which class it should be.

I wonder if this is possible depending of the condition evaluated.

@gaogaotiantian
Copy link
Member

Oh sorry I did not look at the PR thoroughly. I saw the word "regex" and thought it was improving the regex method.

classes do not have useful compile time information with it as far as I know which makes this tricky - that's why I said we probably need some heuristics. I doubt if we can make this work:

if runtime_cond:
    class A:
        pass
else:
    class A:
        pass

They are indistinguishable. We can, however, try to figure out which class it is by some "guessing" like reading their methods. Let me know if this is out of your comfortable zone and you'd like me to work on it, I'm kind of familiar with the area. Otherwise, let me know if you have any question about the implementation.

@Viicos
Copy link
Contributor Author

Viicos commented Jul 15, 2023

I'll try to take a look, I'm not that familiar with ast but I'll try to come up with something (this is a good opportunity to learn more about it), we will then be able to work on this together if I encounter any blocking points

@Viicos
Copy link
Contributor Author

Viicos commented Jul 16, 2023

Actually while I see how this could be implemented, I don't really see which attributes should be used to guess the right class. @gaogaotiantian if you have something in mind I'll be pleased to see a PR

@gaogaotiantian
Copy link
Member

No worries, I'll work on it, it can be a bit tricky.

@Viicos
Copy link
Contributor Author

Viicos commented Jul 16, 2023

No worries, I'll work on it, it can be a bit tricky.

Yeah I was first thinking maybe use the values from __dict__, but we have to make sure not to include attributes set by a potential metaclass

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants