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

objgraph should not merge distinct classes with same name #4

Closed
mgedmin opened this issue Jan 25, 2014 · 4 comments
Closed

objgraph should not merge distinct classes with same name #4

mgedmin opened this issue Jan 25, 2014 · 4 comments

Comments

@mgedmin
Copy link
Owner

mgedmin commented Jan 25, 2014

Forwarded from LaunchPad #1183768:

Classes in different may have same class name, like

module.a.A
module.b.A

But typestats merges these objects into one entry key "A", making the result of show_growth confusing.

I think it's better to use the full qualified name of a class as key is better,
{
"module.a.A": 12,
"module.b.A": 3
}

@mgedmin
Copy link
Owner Author

mgedmin commented Jan 25, 2014

Hm. I see people out there actually use the typestats() function. I'd hate to make a backwards-incompatible change and break their code.

How about this: typestats() gets a new argument, shortnames=True. Functions wrapping typestats(), like show_growth, get the same argument. Then perhaps in objgraph 2.0 I break backwards-compatibility by switching the default value to False.

@mgedmin
Copy link
Owner Author

mgedmin commented Jan 25, 2014

Here's a list of all the APIs that should be able to distinguish foo.A from bar.A

  • typestats(shortnames=False)
  • most_common_types(shortnames=False)
  • show_most_common_types(shortnames=False)
  • show_growth(shortnames=False)
  • count('foo.A') -- should probably support both short names and full names
  • by_type('foo.A') -- likewise
  • show_refs(shortnames=False) -- should be able to display them differently
  • show_backrefs(shortnames=False) -- likewise
  • show_chain(shortnames=False) -- likewise

I'm thinking for count()/by_type() I could check if the argument contains a '.', can compare the fully-qualified typename; if it doesn't, I use the current logic of using type(o).__name__.

I wonder if I can write a custom class with name that contains a dot. Oh well, evil people who do that can use by_type('module.Evil.Class').

I wonder if Python 3 __qualname__ is relevant here.

@mgedmin
Copy link
Owner Author

mgedmin commented Jan 26, 2014

I'm pushing my WIP to the long-typenames branch.

I will probably rebase it interactively a few times before merging.

@mgedmin
Copy link
Owner Author

mgedmin commented Jan 26, 2014

I merged the branch to master and deleted it.

@mgedmin mgedmin closed this as completed Jan 26, 2014
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

1 participant