Skip to content

fix(profiling): Resolve inherited method class names#1756

Merged
Zylphrex merged 1 commit intomasterfrom
txiao/fix/resolve-inherited-method-class-names
Nov 29, 2022
Merged

fix(profiling): Resolve inherited method class names#1756
Zylphrex merged 1 commit intomasterfrom
txiao/fix/resolve-inherited-method-class-names

Conversation

@Zylphrex
Copy link
Copy Markdown
Member

Methods may be inherited from a parent class. If multiple classes inherit from the same class and uses the inherited method, we'd want it to report the parent class's name instead of the individual child classes since they'd have the same filename and lineno of the parent class and not the children.

Methods may be inherited from a parent class. If multiple classes inherit from
the same class and uses the inherited method, we'd want it to report the parent
class's name instead of the individual child classes since they'd have the same
filename and lineno of the parent class and not the children.
@sl0thentr0py
Copy link
Copy Markdown
Member

sl0thentr0py commented Nov 29, 2022

just a footnote:
I'm wondering if we're missing some standardization in extracting meaningful names from frames, all of this seems like extra logic that should probably live somewhere more general upstream?

@Zylphrex
Copy link
Copy Markdown
Member Author

just a footnote: I'm wondering if we're missing some standardization in extracting meaningful names from frames, all of this seems like extra logic that should probably live somewhere more general upstream?

Yeah, this could potentially be something we port over to errors as well since names like __init__ isn't particularly meaningful without the class name. This was a major pain point when looking at profiles but if this can enhance the experience for errors, I think it makes sense for this logic to be more general.

@Zylphrex Zylphrex merged commit 1c886e6 into master Nov 29, 2022
@Zylphrex Zylphrex deleted the txiao/fix/resolve-inherited-method-class-names branch November 29, 2022 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants