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

Model fields and type hints #109

Closed
felix-hilden opened this issue Dec 28, 2019 · 14 comments
Closed

Model fields and type hints #109

felix-hilden opened this issue Dec 28, 2019 · 14 comments
Labels
documentation Improvements or additions to documentation waiting Waiting for another event

Comments

@felix-hilden
Copy link
Owner

felix-hilden commented Dec 28, 2019

Dataclass fields, and subsequently type hints are not shown on the documentation (example). It would be very useful to view them and even jump to other definitions with similar links that are available elsewhere.

@felix-hilden felix-hilden added the documentation Improvements or additions to documentation label Dec 28, 2019
@felix-hilden
Copy link
Owner Author

Not using sphinx_autodoc_typehints makes the types appear in the constructor documentation, but it's very messy. Doc comments after every attribute like #: foo results in attribute = None and the comment. But :class:'TypeHint' isn't parsed correctly. A related issue.

@felix-hilden felix-hilden added the help wanted Extra attention is needed label Jan 3, 2020
@felix-hilden
Copy link
Owner Author

We may finally have what we are looking for in Sphinx 3! It introduced a new extension to autodoc, which has similar functionality to the 3rd party (although popular) autodoc_typehints.

Doc comments don't quite work, and if we were to document attributes, they won't inherit from a dataclass to another. Additionally, the type hints show the full name of the object, e.g. tekore.model.Paging, which is not ideal. But I'll look into this.

@felix-hilden
Copy link
Owner Author

felix-hilden commented Jun 7, 2020

We could indeed start using the builtin extension fully. It is essentially enabled by disabling the 3rd-party autodoc typehints and specifying two options:

autodoc_typehints = 'description'
autoclass_content = 'both'

Here are some problems that need to be resolved:

Most of them seem like issues with or additions to Sphinx itself, so maybe we ought to wait a bit and make the change once the issues have been resolved. Seems like Sphinx 3.2 might be the version when all this is resolved.

@felix-hilden felix-hilden added waiting Waiting for another event and removed help wanted Extra attention is needed labels Jun 7, 2020
@felix-hilden
Copy link
Owner Author

With this shift, we could also have a look at can Napoleon and Autodoc be used to document function return types as well. Currently the Numpy style requires the return type to be present, but it shouldn't need to be so. An issue: sphinx-doc/sphinx#7077

This can be alleviated by using :return:, but that completely misses the point of using Numpy documentation.

# Signature
def foo(...) -> Tuple[type, another]

# Numpy docstring
"""
Returns
-------
Tuple[type, another]
    this returns things
"""

# Hack around napoleon
"""
:return: this returns things
"""

@HarrySky
Copy link
Contributor

About type hints - it would be great to add py.typed file to package (see PEP 561) 😄
It will show mypy that this package has type hints and it will give more helpful analysis.

Easy PR:

  1. Create empty file tekore/py.typed
  2. Add line to MANIFEST.in: include tekore/py.typed

@MarkoShiva
Copy link

Getting errors from the spotify query with your hack in the serialise.py for Json track list.

@felix-hilden
Copy link
Owner Author

felix-hilden commented Nov 17, 2020

I'm not sure what you are referring to, @MarkoShiva. If this is a bug, would you mind opening a dedicated issue with some more information? Which spotify query? What hack? Which errors?

@felix-hilden
Copy link
Owner Author

An update: due to other changes in Tekore, inheriting the docstrings of other libraries is no longer an issue. I don't think we have any inheritance from other libraries now. Turning off fully qualified names was fixed recently, but some discussion of it still not working is ongoing. If that issue's ok, then we are left with just properties missing their return type, which seems to be delayed until at least Sphinx version 4. Where it hurts us most is in Token documentation, but we could specify the types manually.

@felix-hilden
Copy link
Owner Author

And recently the support for property return type hints was added too! This all is to be expected in Sphinx 4.0, released in April, so let's look at changing the doc configuration and finally resolving this issue then.

@felix-hilden
Copy link
Owner Author

Sphinx 4 was released just now, but it seems that the newly introduced option python_use_unqualified_type_names doesn't work quite as expected. The documentation still contains a ton of fully qualified references. Also, some Optional hints are dropped. I'll have to investigate some more.

@felix-hilden
Copy link
Owner Author

felix-hilden commented May 31, 2021

Just for reference, I've opened sphinx-doc/sphinx#9268. Edit: now closed and waiting for a release on the 12th.

@felix-hilden
Copy link
Owner Author

I've now pushed the type hint improvements. The model parameters are now also documented in the description, which would allow for us to document the fields, but the docs are not inherited. We should maybe look into a solution for that as well. Maybe Attrs?

@felix-hilden
Copy link
Owner Author

It seems too difficult to really be worth it. The attributes are documented in the Spotify Object Model. We should link to it, but that ought to be enough.

@felix-hilden
Copy link
Owner Author

Done in a0e8491

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation waiting Waiting for another event
Projects
None yet
Development

No branches or pull requests

3 participants