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

Re-exporting not working when using wildcard import in __init__.py #565

Closed
buhtz opened this issue May 7, 2022 · 11 comments · Fixed by #572
Closed

Re-exporting not working when using wildcard import in __init__.py #565

buhtz opened this issue May 7, 2022 · 11 comments · Fixed by #572
Milestone

Comments

@buhtz
Copy link
Contributor

buhtz commented May 7, 2022

This is about Re-exporting as described in the documentation.

I have a function mypackage._mypackage.foo and a class mypackage._mypackage.Bar. My goal is that they appear in the documentation as mypackage.foo and mypackage.Bar. But currently they don't. Please see the attach minimal example including the generate HTML docu: pydbug.zip.

__init__.py

As described in the documentation I try to use the __all__ variable.

from ._mypackage import *

__all__ = ['foo', 'Bar']

But it has no effect.

Sidenote

  • The package was installed via pip3 install -e ..
  • I call pydoctor via my script docs/hey_doc.py. It simply extract some meta-data from the package and use it as comandline arguments.
  • Using Debian 11 (current stable) with Python 3.9.2 and pydoctor 22.5.1.

EDIT

In my example I do an import *.
In your documentation about Re-exporting you explicit import all elements.
So when I do from ._mypackage import foo, Bar it works. In that case there is also a message about it in the generator output.

moving 'mypackage._mypackage.foo' into 'mypackage'
moving 'mypackage._mypackage.Bar' into 'mypackage'

OK, now we know some more.

I see no choice to explict import all components. I wan' everything and all for all the time. I don't want to think about to import something explicit when I create a new function or class in _mypackage.py. I know that PEP8 tells to avoid wildcard imports like from module import *. But in that case it makes sense.

@buhtz buhtz changed the title Re-exporting not working Re-exporting not working when using wildcard import in __init__.py May 7, 2022
@tristanlatr
Copy link
Contributor

Hello @Codeberg-AsGithubAlternative-buhtz, thank you very much for your interest into pydoctor, and all the propositions that you do.

With this issue, your scratching the limits of our current ast builder. Analyzing wildcard import is something highly complex, that astroid doesn’t even get right. I’m talking about astroid because I’m looking at this library to implement a new ast builder. See #430. But maybe something can be done with the current builder…

Solving this issue the right way is not obvious. And I agree it would be good that pydoctor recognize such constructs.

What you could do to help out is to write a test case that is expected to fail for now. Then you can look at the ast builder code around the visit_ImportFrom function to see the re exporting logic, in case you want to give it a go an implementing something.

Talk to you later,

@buhtz
Copy link
Contributor Author

buhtz commented May 7, 2022

I am sorry I am not deep enough into AST or your code to contribute here.
But I know that Doxygen can handle wildcard imports. But most of it was written in C.

@buhtz
Copy link
Contributor Author

buhtz commented May 7, 2022

And another detail. In the below code I fill the __all__ automatically based on dir() to prevent me from typing redundant things. This worked fine with Doxygen but not with pydoctor.

from ._mypackage import foo, Bar

__all__ = []

for v in dir():
    if not v.startswith('__'):
        __all__.append(v)

Only this works

from ._mypackage import foo, Bar
__all__ = ['foo', 'Bar']

@tristanlatr
Copy link
Contributor

This worked fine with Doxygen but not with pydoctor.

It is highly complex to recognize such construct with static analysis.

And actually, I'de argue this is not very pythonic. You are basically including everything in your __all__ variable, which is almost the same as not mentioning __all__ variable at all. I'de consider this as a code smell, maybe your package can be refactored not to use __all__ variable at all?

@buhtz
Copy link
Contributor Author

buhtz commented May 7, 2022

In my understanding I don't need the __all__ variable but pydoctor does. Without using it foo and Bar are not moved from mypackage._mypackage to mypackage.

Maybe I missunderstood something?

@tristanlatr
Copy link
Contributor

Maybe I missunderstood something?

No, you're right that the names should be part of the __all__ variable if we want pydoctor to be able to export it.

Though, I think we can do something about this kind of construct:

from ._mypackage import *

__all__ = ['foo', 'Bar']

But the names will still have to be statically listed as strings to be recognized by pydoctor.

@buhtz
Copy link
Contributor Author

buhtz commented May 8, 2022

Though, I think we can do something about this kind of construct:

from ._mypackage import *
__all__ = ['foo', 'Bar']

But the names will still have to be statically listed as strings to be recognized by pydoctor.

This would help a lot.

But what is still mysterious for me is that this construct works in Doxygen but not in pydoctor:

__all__ = []
for v in dir():
    if not v.startswith('__'):
        __all__.append(v)

I also did some debug testing like this

__all__ = ['foo', 'Bar']
all = []
for v in dir():
    if not v.startswith('__'):
        all.append(v)
print(all == __all__)  # result is True

I assume that the call of dir() did something. Or does pydoctor parse the py-file as a text file?

@tristanlatr
Copy link
Contributor

Or does pydoctor parse the py-file as a text file?

Yes, pydoctor parses the AST of your source files. This is why is so complicated to infer these kind of stuff. Because pydoctor doesn’t actually import your module.

@tristanlatr
Copy link
Contributor

But I’ll look at adding better support for wildcard.

@buhtz
Copy link
Contributor Author

buhtz commented May 8, 2022

Yes, pydoctor parses the AST of your source files. This is why is so complicated to infer these kind of stuff. Because pydoctor doesn’t actually import your module.

I read a bit about AST but no sure about the practical consequences.
pydoctor does not read the content/value of __all__ but read the source line (via AST) where the value is set? Interesting.

When I do from ._mypackage import * you do know which py-file corosponds to _mypackage. Can't you then "read" the AST of that file and just include everything (because of the wildcard) from it?

@tristanlatr
Copy link
Contributor

pydoctor does not read the content/value of __all__ but read the source line (via AST) where the value is set?

Yes

Can't you then "read" the AST of that file and just include everything (because of the wildcard) from it?

Yes also, we have some support for wildcard. I’m just unsure if the reexport logic take into account these kind of imports. I’ll try to have a look soon.

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

Successfully merging a pull request may close this issue.

2 participants