JelleZijlstra / typeshed_client

Retrieve information from typeshed and other typing stubs
MIT License
20 stars 2 forks source link

get_fully_qualified_name doesn't work for nested classes #80

Open dfremont opened 1 year ago

dfremont commented 1 year ago

For example,

import typeshed_client
resolver = typeshed_client.Resolver()
resolver.get_fully_qualified_name('imaplib.IMAP4')

works but

resolver.get_fully_qualified_name('imaplib.IMAP4.readonly')

does not. It's possible this is intended behavior, since you can get the definition for the imaplib.IMAP4.readonly class with

resolver.get_fully_qualified_name('imaplib.IMAP4').child_nodes['readonly']

but this behavior is unintuitive to me (why should it matter whether the class is nested or not?), so if it is by design then it would be helpful to add a note to the documentation saying get_fully_qualified_name only works for top-level names.

(In fact it would be nice if get_fully_qualified_name worked for methods, etc. too so that anything defined in typeshed could be looked up directly without having to sometimes traverse through child_nodes. For example one could write get_fully_qualified_name(f'{func.__module__}.{func.__qualname__}') to see if there is a definition for a function or method func. If there's interest in that I can try to put together a PR.)

JelleZijlstra commented 1 year ago

This would complicate the implementation because right now we can assume that only the part after the last dot is a module member, and everything before it is a module. So we can simply split by dot, use the first n-1 parts as the module path, and find the last one in the module we find.

But with your desired behavior, the necessary split may be anywhere in the string, since there may be arbitrary levels of nesting. The result may even be ambiguous, as there could be both an imaplib.IMAP4 class and an imaplib.IMAP4 submodule that both contain a readonly member. (I am fairly confident this will really happen for some third-party packages.)

So I would be hesitant to make get_fully_qualified_name behave this way. However, we could add a separate API with two parameters, one for the module and one for the path within the module.

dfremont commented 1 year ago

Ah, the ambiguous case hadn't occurred to me. So perhaps it would be best to just add a note to the documentation clarifying what get_fully_qualified_name currently does.

If you'd like to include a separate API of the kind you suggest, I'm happy to make a PR from my current code (which is quite simple).

JelleZijlstra commented 1 year ago

I'd approve adding a new separate API, thanks!