-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Fix Typing #3086
Merged
Merged
Fix Typing #3086
Changes from 1 commit
Commits
Show all changes
101 commits
Select commit
Hold shift + click to select a range
bd4d123
first draft of color class + starting library conversion
MrDiver 01e28fd
Merge branch 'main' into color_fix
MrDiver a0e85af
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 8af3f81
Merge branch 'main' into color_fix
MrDiver 332d327
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] da0806a
changed everything to Manim color todo: figure out circular dependenc…
MrDiver 73ffb51
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 157c504
first working draft of new color version
MrDiver a7e4a3e
Merge branch 'main' into color_fix
MrDiver d21583d
resolving conflicts
MrDiver 074345e
resolving conflicts
MrDiver 128b8ab
resolving conflicts
MrDiver 31bcdac
resolving conflicts
MrDiver 27c0ddb
resolving conflicts
MrDiver 195512a
changed default internal value of ManimColor to np.ndarray[float]
MrDiver 9823616
starting to fix tests
MrDiver e638643
fixed more tests and changed precision of manim color
MrDiver 7867aa3
removed premature color conversion
MrDiver a12507a
fixed some more tests
MrDiver 15c4998
final test changes
MrDiver 76d7cda
fix doctests
MrDiver a27aff4
fix for 3.8
MrDiver 1580c82
fixing ManimColor string representation
MrDiver b2cc282
removing some unneccesary conversions
MrDiver 2baeff5
moved community constants to manim_colors.py and added more color sta…
MrDiver 86c5d07
Merge branch 'main' into color_fix
MrDiver b44a3e9
Merge branch 'color_fix' of github.com:MrDiver/manim into 52-fix-typing
MrDiver cd7a385
Added typing.py and typed bezier.py, core.py, constants.py fully
MrDiver 47e16b6
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 3193597
fixed codeql complaints
MrDiver bd3bf0c
add type ignore for np.allclose
MrDiver 8c1b871
fixed import in three_dimensions
MrDiver 2433c1d
added ignore for F401 back again in flake
MrDiver d32b267
added typings to coordinate_systems.py
alembcke 0356591
Few improvements to `graphing/coordinate_systems.py`
Viicos b08e4a6
added some typings to mobject/geometry/line.py
alembcke ee865ff
updated typings for mobject/geometry/line.py
alembcke 9eb6ed3
Add missing imports to `line.py`
Viicos 561ef52
added typings to three_dimensions.py
alembcke bf2bcbf
Use `FunctionOverride` for animation overrides
Viicos 1f56200
Remove `TYPE_CHECKING` check
Viicos 802ab71
Revert "Remove `TYPE_CHECKING` check"
Viicos b11ffe2
Use `Self` in `coordinate_systems.py`
Viicos a41900c
Typehinted mobject.py and updated manim.typing.py
JasonGrace2282 c0f121c
Typed VMobject
JasonGrace2282 88f9b2d
Type-hinted manim.mobject.geometry
JasonGrace2282 17ce647
math.cos->np.cos, etc & fixed incorrect typehints
JasonGrace2282 747167b
Merge branch 'main' of github.com:ManimCommunity/manim into 52-fix-ty…
MrDiver 2a71462
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 8a149ca
fix missing annotations import
MrDiver 28d9b24
Merge branch '52-fix-typing' of github.com:MrDiver/manim into 52-fix-…
MrDiver 6987498
TypeAlias fix in typing.py
MrDiver 9026015
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 51922d1
Add ignore errors again to mypy because commits are not possible like…
MrDiver e8ab923
Merge main
Viicos 106774d
Fix last typing issues
Viicos ca3154c
Update docs
Viicos 920d15c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] c6183e8
Only type check manim
Viicos ad7043f
Try fixing pre-commit
Viicos 1da1b99
fix merge
Viicos 8522b59
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 6f95fd3
Fix compat
Viicos c2d7d42
Fix compat again
Viicos 3a3d501
Fix imports compat
Viicos 2189d01
Use union syntax
Viicos e62ade2
Use union syntax
Viicos 4497734
Fix reduce_across_dimension
Viicos aadb22c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 0852403
Various test and merge fixes
Viicos b9fbf54
Doc fixes
Viicos 0f108c0
Last doc fix
Viicos 5160a1f
Revert usage of np over math
Viicos 388b6d1
Bump numpy version
Viicos 287c3ad
Remove obsolete duplicate example
Viicos 0c0f875
Fixed Incorrect Typehint in manim.constants
JasonGrace2282 b87b594
Fix docstring typo
Viicos 1c139af
More fixes
Viicos ffaa031
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 56501d7
docs fixes
Viicos 921f450
Add internal aliases
Viicos 4e3fe30
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 5663791
fix compat
Viicos f92ebd6
Merge remote-tracking branch 'origin/main' into 52-fix-typing
behackl feca683
line lengths in .rst file, formatting, typos
behackl 2bcd8ac
add docstring for space_ops:cross2d
behackl 5514ac4
add some more arrow tip typings (in a non-circular import causing way)
behackl b259a04
yes, this can be deleted
behackl e390082
fix formatting of example
behackl 737189c
added docstring to bezier::inverse_interpolation
behackl 3c4051f
added docstring + test for bezier::match_interpolate
behackl c7659d3
some improvements in coordinate_systems
behackl 351353a
Vector -> Vector3
behackl 3616c50
replaced np.ndarray with more appropriate type hints
behackl 672b0e3
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 84c9cdc
Apply feedback
Viicos 4fed4af
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] ad228f1
revert to previous (new) version
behackl c2b48f8
fix doctest
behackl 948214e
fix ReST errors
behackl 33d9e4a
Merge remote-tracking branch 'origin/main' into 52-fix-typing
behackl File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think
Never
is used correctly here 🤔 It is usually not the way to type hint functions that can possibly raiseThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then how should one type a function that is supposed to raise an
Exception
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usually a function that always raise uses
NoReturn
, but if it raises conditionally then you can't really tell with type hints (some docstring conventions such as the one from Google have a dedicated section).You would probably want to use
NoReturn | None
, but iirc mypy isn't happy with it, and having this doesn't add any value to the return type imo 🙂There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So then I'll just keep it as
-> None
in my next push :)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! I would love to continue contributing in typing but I'm still concerned about #3086 (comment). Unfortunately I'm missing time currently, but I'll see if I can still contribute a bit
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it should be fine for now, there is not that much left to do, we already got the Colors out of the windows now and what's left is currently not doable because of recursive imports so the next thing that is merged will be this PR and probably nothing else so we have a clean slate of manim that we can work on for the other PRs which makes a lot more sense then.