Manim: Add typings

Created on 21 May 2020  路  15Comments  路  Source: ManimCommunity/manim

We should add typings to manim:

  1. It allows for more precise intellisense and easier-to-check code (via mypy and whatnot);
  2. It ensures we follow a structure that makes sense (i.e., ensures we don't have a data model in which people get completely lost);
  3. Writing documentation and specifying types are tied to each other. When we work on documentation, it is inevitable that we will specify types for things, so why not add it to the code as well?

Notes:

  1. We can use stubgen to have a headstart;
  2. We can use stub files in order to keep typings separate from the code and not have to change things much (but this is debatable - please expose your opinions on this);
  3. This is very related to the dataclasses solution proposed in #7, and may ultimately be a consequence of it.

Thoughts?

documentation enhancement new feature

Most helpful comment

@PgBiel I meant having in-code _and not having_ in-docstring.

got it

What is the benefit of having both? All I could think of is that a user reading the docs might not get the full benefit from in-code, but if Sphinx can be configured to parse in-code and show those to the user, then why bother?

if we can configure that, then it should be fine
The only thing that we have to note is that, at the moment, we aren't adding typehints in code, so they must be added in docstring for now
Then a future typings PR may move them from the docstring to the code, and we may have simpler docstring types.

All 15 comments

One of the main problems with this is contributions from the community. What if a user wants to contribute a PR but they don't have experience with typing? Do we just reject all PRs without types, or do we guide each user in adding types? Or do we merge them and add the types ourselves?

The latter is more appropriate. In general, the one making the PR should attempt to write the typings themselves. If they do not manage to, we can work this out by ourselves. (Not sure if the usage of stubs would prove itself more useful in this case)

^we should at least expect PRs to have documentation, which makes writing typings by ourselves trivial

^we should at least expect PRs to have documentation, which makes writing typings by ourselves trivial

Good point.

It seems like we have a pretty solid plan for this. Should we get started?

I think this should be left until after we are done with the "initial clean up" milestone.

this should be done in conjunction with the dataclasses change for optimal compatibility and development.

The latest developments in #7 kinda sound like we won't be using dataclasses. If that's the case, then we can start adding typings at any time, say one module at a time. Any takers?

I think this is a good place to continue the discussion from https://github.com/ManimCommunity/manim/pull/483#discussion_r503470863

The points I was trying to make:

  • Type hints (PEP 484) are an excellent idea, I am all for adding them! A module-by-module approach sounds good to me, and I'd be happy to help implementing it.
  • Duplicating information (having typing information both in the form of type hints in method signatures as well as in docstrings) is a really bad idea; it will inevitably introduce inconsistencies.
  • Sphinx is able to deal with type hints and can format the html documentation so that it looks the same as rendering Numpy-style typing information in the docstring.
  • Type hints are picked up by modern IDEs (!)

I'd like to suggest changing our policy from adding typings via docstrings to adding typings via type hints. :-)

Using in-code typings ~vs~ _instead of_ in-docstring typings sounds like a good idea generally TBH.

Using in-code typings vs in-docstring typings sounds like a good idea generally TBH.

Do you mean having typings in both of them, or separating them?

I'd like to suggest changing our policy from adding typings via docstrings to adding typings via type hints. :-)

As I said before: we may do that, however only after we actually get to work on in-code typehints. Until then, we need something to base ourselves on in order to generate typings.

@PgBiel I meant having in-code and not having in-docstring.

What is the benefit of having both? All I could think of is that a user reading the docs might not get the full benefit from in-code, but if Sphinx can be configured to parse in-code and show those to the user, then why bother?

Are we talking something like this:
https://realpython.com/python39-new-features/#annotated-type-hints
where you add both documentation about a function argument as well as its type information as an annotation in the function signature?

@PgBiel I meant having in-code _and not having_ in-docstring.

got it

What is the benefit of having both? All I could think of is that a user reading the docs might not get the full benefit from in-code, but if Sphinx can be configured to parse in-code and show those to the user, then why bother?

if we can configure that, then it should be fine
The only thing that we have to note is that, at the moment, we aren't adding typehints in code, so they must be added in docstring for now
Then a future typings PR may move them from the docstring to the code, and we may have simpler docstring types.

Are we talking something like this:
https://realpython.com/python39-new-features/#annotated-type-hints
where you add both documentation about a function argument as well as its type information as an annotation in the function signature?

no no, this style of annotation was before the latest typing PEP took place, which ensured that annotations should be _exclusively_ for typings (starting on py 3.5). We are talking about reproducing the typehint syntax in the documentation string of the class/function/... when specifying what a parameter or attribute is, for example.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

huguesdevimeux picture huguesdevimeux  路  7Comments

cobordism picture cobordism  路  5Comments

nilaybhatia picture nilaybhatia  路  7Comments

qo4on picture qo4on  路  6Comments

huguesdevimeux picture huguesdevimeux  路  5Comments