Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Type Guide either in the Documentation or in a readme file. #2765

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

Closed
MrDiver opened this issue Jun 12, 2022 · 1 comment
Closed

Type Guide either in the Documentation or in a readme file. #2765

MrDiver opened this issue Jun 12, 2022 · 1 comment
Labels
documentation Improvements or additions to documentation help wanted We would appreciate help on this issue/PR Suggestion Requesting a feature or change for Manim

Comments

@MrDiver
Copy link
Collaborator

MrDiver commented Jun 12, 2022

Description of proposed feature

There should be a clear type guide for which types to prefer in manim. There are a lot of ways to type things for example Optional[int] and int | None are the same thing when considering a linter like mypy, but they convey a different story when reading. For parameters if something has the type Optional it's very clear that this parameter can be left out safely and the rest is handled by the function itself.
This also reflects itself inside the function when reading the typehints, where the error messages state that the type is Optional and thus things like x.something don't work. This is also the case with int | None (it translates to Union[int, None] but it suggests that this variable can be in general a None value and also might just be changed back to one during the lifetime of the Object.

That's why i think we need to discuss a clear typeguide for manim and also have some kind of suggestions and help regarding typing.

I also would suggest the addition of a type module for Custom types needed in the future like the Color type which currently is str | Color, but this might be an addition for another day. But could also be discussed here if we need some standard types.

V1 V2 We use!
Optional[T] T | None -
ndarray NDArray[dtype] -
List[T] list[T] -
Type[cls] type[cls] -
Union[T,K] T | K -

New Types in question

  • TimeBasedUpdater = Callable[["Mobject", float], None]
  • NonTimeUpdater = Callable[["Mobject"], None]
  • Updater = Union[TimeBasedUpdater, NonTimeUpdater]
  • ManimColor = Union[str, Color]

Related to

@MrDiver MrDiver added documentation Improvements or additions to documentation help wanted We would appreciate help on this issue/PR labels Jun 12, 2022
@behackl
Copy link
Member

behackl commented Jun 12, 2022

It would be very nice to have a dedicated manim.typing module where we can (consistently) write more complicated types and then just use them in the rest of the library.

In terms of the semantics between Optional[bla] vs. bla | None: do you have any references for how other projects handle this? Personally, I don't have a strong opinion on caring about semantics between the two variants.

@MrDiver MrDiver added the Suggestion Requesting a feature or change for Manim label Jun 16, 2022
@ManimCommunity ManimCommunity locked and limited conversation to collaborators Jun 16, 2022
@MrDiver MrDiver converted this issue into discussion #2822 Jun 16, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
documentation Improvements or additions to documentation help wanted We would appreciate help on this issue/PR Suggestion Requesting a feature or change for Manim
Projects
None yet
Development

No branches or pull requests

2 participants