sphinx icon indicating copy to clipboard operation
sphinx copied to clipboard

Added support for reading .pyi files.

Open alliefitter opened this issue 7 years ago • 40 comments

Subject: Added support for reading .pyi files

Feature or Bugfix

  • Feature

Purpose

I created a class that dynamically builds its methods at runtime and wanted to create some docs for it. So, I made a .pyi file with the dynamic methods defined and with docstrings. PyCharm was able to read this file and provide code completion and documentation, but sphinx required a little changing. The change in this commit is more of a proof of concept than anything. I REALLY don't know this codebase or how best to incorporate this--as a change to sphinx or as an extension. I'd really appreciate any thoughts or guidance on what I can do to expose this functionality.

Detail

  • When importing a module, checks if a .pyi file exists by using os.path.isfile('{}i'.format(module.__file__)).
  • Imports the .pyi file using importlib.
  • Replaces the original module with the one imported from the .pyi file.
  • From testing, sphinx doesn't seem to have any issues using the object imported from .pyi file.

alliefitter avatar Apr 09 '18 05:04 alliefitter

What is .pyi file? Is it standardly used?

tk0miya avatar Apr 09 '18 17:04 tk0miya

It's a stub file introduced in PEP 484. Meant so you can use typing to add annotations without breaking in pre-python 3.5. See also typeshed which is included with PyCharm.

alliefitter avatar Apr 09 '18 20:04 alliefitter

You guys have an open issue that references .pyi files in #2755.

alliefitter avatar Apr 09 '18 20:04 alliefitter

Thank you for description. I understand. I know stub files for mypy (typeshed). But I've never know PyCharm using it as external type hinting. https://www.jetbrains.com/help/pycharm/type-hinting-in-product.html#pep484

+0 for this for following reasons:

  • I think the external type hinting is not needed for py3.
  • On the other hand, at present, it is useful to annotate types for py2 code.
  • Until the EOF of py2, the proposed feature seems enough useful to me.
  • I don't know why external docstrings is needed. So I can't say it is needed or not.

I think this is temporarily needed feature. So it would be nice if we can implement this as an extension. I'll think about it later.

tk0miya avatar Apr 10 '18 14:04 tk0miya

Thanks for the consideration. I was leaning towards an extension myself. I'll have to get back to it when I have some time to figure how best to implement it. Also just one more use case for this (though it is pretty narrow) is an object whose methods defined until run time. As I said this is why I needed to experiment with this. Basically, the methods are decided using the strategy pattern when the object is instantiated. The stub files allowed me to define the signature of the methods and add docstrings for the default strategy.

alliefitter avatar Apr 10 '18 16:04 alliefitter

I feel your case is not usual. So I can't determine to add this feature to sphinx-core.

As a workaround, you can override import_module() function like following:

# in your extension
from sphinx.ext.autodoc import importer

# keep original import_module()
original_import_module = importer.import_module

def import_module(modname, warningiserror=False):
    module = original_import_module(modname, warningsiserror)
    if hasattr(module, '__file__') and os.path.isfile('{}i'.format(module.__file__)):
        ...  # merge external spec into the imported module

# override import_module() by own
importer.import_module = import_module

I know this is very hacky. But I believe this will work without changing sphinx-core.

tk0miya avatar Apr 14 '18 08:04 tk0miya

@shimizukawa any comments?

tk0miya avatar Apr 14 '18 08:04 tk0miya

  • Apparently, this code does not seem to work with Py2.
  • As @tk0miya said, in order to incorporate this feature into sphinx-core, we need a general case of reading the pyi file in the same directory and doing autodoc.

shimizukawa avatar May 06 '18 13:05 shimizukawa

@shimizukawa Thank you for comment.

@alliefitter I markded this as "wontfix". I think there are no proper way. But we can hack it with workaround. So please use it for your extension. Thanks,

tk0miya avatar May 06 '18 14:05 tk0miya

I just stumbled across this issue by googling and have exactly the same problem as OP. I also created some methods dynamically and documented them using .pyi files. I was very much assuming that Sphinx would use them to generate documentation, as many typing-related things are also supported. Are you sure this is out of scope and a special case? I'd love to see this included in one way or another, as there really is a valid use case even in Python3.5+.

@tk0miya

I think the external type hinting is not needed for py3. I don't know why external docstrings is needed. So I can't say it is needed or not.

For dynamically generated types, functions and methods, you really don't have any other way of annotating them properly. Could someone hint me to some docs or examples on how you could override the internal behavior of the importer by using an extension?

JosXa avatar May 06 '18 17:05 JosXa

@JosXa

Could you let me know about "many typing-related things"? I'd like to know this is commonly used way or not. Is there any specification?

Could someone hint me to some docs or examples on how you could override the internal behavior of the importer by using an extension?

Read my comment above please: https://github.com/sphinx-doc/sphinx/pull/4824#issuecomment-381314113

tk0miya avatar May 07 '18 12:05 tk0miya

I apologize for not responding sooner. I get a flood of Github notifications from work everyday and missed these notifications. @JosXa glad that some else has a need for this besides me. @tk0miya I agree that this doesn't belong in sphinx-core. @shimizukawa inherently this isn't compatible with 2.7 because the typing module is 3.5+. Which is another reason why this should be an extension.

With @tk0miya 's workaround, I think I can get this done over the weekend. Thanks, guys!

alliefitter avatar May 07 '18 16:05 alliefitter

Oh, and @JosXa, just a heads up. If you're using the Google Docstring extension, you have to edit that too in order for it work properly.

alliefitter avatar May 07 '18 16:05 alliefitter

many typing-related things

Sorry, I'm not very experienced with Sphinx as this is the first project I'm making public docs for, so I'm not exactly sure. But all the normal argument type hinting with foo: Type seems to work (as supported by google docstrings?), so I thought .pyi stubs should too.

@tk0miya Ya so I got a custom extension rolling with your guideline, thanks for that. However, it has issues with the non-standard syntax of .pyi stubs, for instance class attributes defined as

class Foo:
    bar: Type

instead of

class Foo:
    bar = Type

I suspect that there might be more instances where the syntax is not quite parseable by a normal python interpreter. Do you not type hint your attributes and properties the same way, @alliefitter? Or have you found a solution to that? I guess it should be fairly simple to script just this one part, but it makes me think that the support for this should come from a more holistic solution than just a workaround/hack.

Is there any specification?

I think the idea stems from the mypi project in order to be able to write and share type hints for third-party libraries that you do not have access to or in order to separate type hints from the source code (PYTHONPATH or shared/typehints/python3.5). Of course there is the issue of code locality, meaning that type hints should usually be right next to the symbols and not in another file, but on the other hand you can also get a lot cleaner projects while still maintaining the autocompletion hints by IDE support (PyCharm). Looking for docs right now...

JosXa avatar May 08 '18 19:05 JosXa

Interesting docs:

Note: Apparently "stub files are written in normal Python 3 syntax, but generally leaving out runtime logic like variable initializers, function bodies, and default arguments, replacing them with ellipses"

JosXa avatar May 08 '18 19:05 JosXa

@JosXa are you using the autodoc typehints extension? I haven't had any issues with the autodoc reading my type annotations.

alliefitter avatar May 08 '18 20:05 alliefitter

Trying it now, and thanks for the link - I included it. Update:

The issue remains:

    bot_under_test: Union[int, str]
                  ^
SyntaxError: invalid syntax

and

    logger: Logger
          ^
SyntaxError: invalid syntax

@alliefitter this is what I'm trying to do: https://github.com/JosXa/tgintegration/blob/freeze/tgintegration/botintegrationclient.pyi#L15

Do you do it differently?

I'm pretty sure it's valid syntax, so something is wrong. Perhaps a too old Python version (edit: no I thinnk 3.5 should be good, right?) or actually an issue with the Sphinx parsing...

JosXa avatar May 08 '18 20:05 JosXa

Yes, it should be fine with 3.5+. I just checked and I'm able to type attributes using that syntax. It could be that you aren't initializing them? Maybe try giving them a default value.

alliefitter avatar May 10 '18 16:05 alliefitter

The mypy documentation, see chapter 2.5 for .pyi stubs

Thanks! This is what I'd wanted. This says we can place .pyi file at same directory as the .py script:

Write a stub file for the library and store it as a .pyi file in the same directory as the library module. http://mypy.readthedocs.io/en/stable/basics.html#library-stubs-and-the-typeshed-repo

It means this is not PyCharm specific behavior. +1 to add this to sphinx-core.

@shimizukawa what do you think?

tk0miya avatar May 13 '18 07:05 tk0miya

Codecov Report

Merging #4824 into master will decrease coverage by 0.02%. The diff coverage is 9.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #4824      +/-   ##
==========================================
- Coverage   82.11%   82.08%   -0.03%     
==========================================
  Files         288      288              
  Lines       38023    38034      +11     
  Branches     5899     5900       +1     
==========================================
  Hits        31221    31221              
- Misses       5485     5495      +10     
- Partials     1317     1318       +1
Impacted Files Coverage Δ
sphinx/ext/autodoc/importer.py 81.94% <9.09%> (-6.03%) :arrow_down:
sphinx/__init__.py 57.5% <0%> (-2.5%) :arrow_down:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b2bd9f7...c6f0a02. Read the comment docs.

codecov[bot] avatar May 13 '18 07:05 codecov[bot]

@tk0miya I take this means I should hold off writing an extension?

@JosXa I was just looking at typeshed noticed this in 2and3.csv:

class Dialect(object):
    delimiter = ...  # type: str
    quotechar = ...  # type: Optional[str]
    escapechar = ...  # type: Optional[str]
    doublequote = ...  # type: bool
    skipinitialspace = ...  # type: bool
    lineterminator = ...  # type: str
    quoting = ...  # type: int
    def __init__(self) -> None: ...

If you're still having trouble you may want to try defining your attributes like this:

bot_under_test: Union[int, str] = ...
logger: Logger = ...

alliefitter avatar May 13 '18 22:05 alliefitter

Now I'm waiting for @shimizukawa 's comment. Until that, it is not determined yet. So I don't think you need to stop developing now. In addition, it will be shipped as Sphinx-1.8 or 2.0 if we'll decide to implement. So it is useful for a while.

tk0miya avatar May 14 '18 13:05 tk0miya

Whether it is for dynamic code or not, I think that it is okay to provide a mechanism for autodoc to get type information from .pyi.

However, documentation is required to include in the release. Unfortunately this feature is only available when running Sphinx with Py3. We need to write such a note in the document. In addition, there is "stub only package" specification in PEP-561. So we also need to write that the feature does not correspond to ".pyi in the stub only package".

shimizukawa avatar May 19 '18 09:05 shimizukawa

@shimizukawa thank you for comment. Okay, let's add this feature to sphinx-core. I'll review this again later.

tk0miya avatar May 20 '18 06:05 tk0miya

Now I'm adding autodoc-before-format-args event to override function signatures from extension. see #6984. I hope this helps to implement this. Is anyone interested in to do that?

tk0miya avatar Jan 11 '20 15:01 tk0miya

My use case is to provide type hints for a native C extension. For a my_module.*.so I have a my_module.pyi sub file that defines the type hints for IDEs. Sadly, autodoc ignores them, and since the functions in my_module.c specify only a loose signature (nothing besides PyObject* args, PyObject* kwargs), apidoc can't generate a proper function signature. Having autodoc consider the pyi stubs would greatly increase the quality of the apidocs in my use case.

hoefling avatar May 07 '20 12:05 hoefling

@hoefling This is a pull request to modify autodoc, not apidoc. If your comment is not related with reviewing this, please post a new issue as a proposal.

tk0miya avatar May 07 '20 16:05 tk0miya

@tk0miya The comment above is related to this PR. Documenting Python C extensions is quite an important use case. It is what the Stub files (*.pyi extension) are mainly targeted for. Not being able to generate docs from it is a deal-breaker for Python binary packages.

Vanuan avatar Aug 30 '20 13:08 Vanuan

Another issue with using sphinx for packages with binary dependencies is that you have to install all the binaries, while only typings are enough.

Vanuan avatar Aug 30 '20 14:08 Vanuan

@alliefitter have you found any workarounds? Is there a patched sphinx version to use in the meantime?

Vanuan avatar Aug 30 '20 14:08 Vanuan