Add missing type hints #1720

gran4 · 2023-04-24T16:26:58Z

Ready to merge. @einarf the errors are either things like variable types undeclared or something I need help on. So pls review. Look at the windows test for the weird error I do not get.

arcade/application.py

arcade/context.py

arcade/gui/property.py

arcade/sprite/sprite.py

Fixes pythonarcade#1743

pushfoo

This is already getting to be a somewhat large PR. I'd turn this change into a separate PR as there may be additional changes related to the ticket.

Pyglet doesn't have __slots__ so I didn't know all the variables

gran4 · 2023-05-03T01:21:06Z

I'll keep those in becuase I don't know how to roll them back, but I'll put anything more into a different PR.
You know what.. ok.
There are a bunch of problems bc using Self.

arcade/gui/property.py

cspotcode · 2023-05-31T21:08:21Z

This should have the latest development branch merged into it.

Looking at the remaining errors:

arcade/math.py:199: error: Need type annotation for "vel"  [var-annotated]
arcade/math.py:237: error: Static methods cannot use Self type  [misc]
arcade/math.py:237: error: A function returning TypeVar should receive at least one argument containing the same TypeVar  [type-var]
arcade/math.py:237: note: Consider using the upper bound "_Vec2" instead
arcade/math.py:239: error: Incompatible return value type (got "_Vec2", expected "Self")  [return-value]
arcade/math.py:242: error: Incompatible return value type (got "_Vec2", expected "Self")  [return-value]
arcade/math.py:245: error: Incompatible return value type (got "_Vec2", expected "Self")  [return-value]
arcade/math.py:248: error: Incompatible return value type (got "_Vec2", expected "Self")  [return-value]
arcade/math.py:251: error: Incompatible return value type (got "_Vec2", expected "Self")  [return-value]

@pushfoo On Vec2, it turns out that using Self as return type is actually wrong. If you subclass Vec2 as MyVec2 and then trigger __add__ via MyVec2(0, 0) + MyVec2(0, 0) then what you get is a Vec2, not a MyVec2.

The correct return type annotation is thus Vec2, not Self. If we think this is a bug in Vec2, then that will happen in a separate PR. The goal for this PR should be for tests and linting to pass, which means changing the annotation to Vec2 or removing the annotation.

arcade/application.py:47: error: Name "Screen" is not defined  [name-defined]

Can be solved by adding Screen to imports: from pyglet.canvas.base import ScreenMode, Screen

arcade/sprite_list/sprite_list.py:1065: error: Item "None" of "Optional[Buffer]" has no attribute "orphan"  [union-attr]
arcade/sprite_list/sprite_list.py:1066: error: Item "None" of "Optional[Buffer]" has no attribute "orphan"  [union-attr]
arcade/sprite_list/sprite_list.py:1067: error: Item "None" of "Optional[Buffer]" has no attribute "orphan"  [union-attr]
arcade/sprite_list/sprite_list.py:1068: error: Item "None" of "Optional[Buffer]" has no attribute "orphan"  [union-attr]
arcade/sprite_list/sprite_list.py:1069: error: Item "None" of "Optional[Buffer]" has no attribute "orphan"  [union-attr]
arcade/shape_list.py:121: error: Item "None" of "Optional[Geometry]" has no attribute "render"  [union-attr]

This class of error is suppressed in our pyright config, so I'm not sure how we want to handle this. Turn off mypy? Turn off union-attr diagnostics? Add assertions? Add # type: ignore?

pushfoo · 2023-06-02T13:30:34Z

The correct return type annotation is thus Vec2, not Self.

Understood & agreed.

If we think this is a bug in Vec2, then that will happen in a separate PR.

It's _Vec2, not Vec2, and that only makes your point stronger. The changes I could suggest to make this more flexible and inheritance safe are irrelevant since:

This is an internal class which only has a single current use: the particle emitter
We might be able to replace the usage with a shader-based particle system or just use pyglet's Vec2.

Even if we don't replace this class and think it's useful, Self is mostly helpful for subclassing. However, having a stable internal class around could be useful to protect against API changes in pyglet's vectors.

This class of error is suppressed in our pyright config, so I'm not sure how we want to handle this.

I'm not sure either. The # type: ignore is awful but it'll get things done quickly and run well. I knowassert has been controversial before, so are any of the other alternatives performant?

gran4 · 2023-06-03T21:31:38Z

I'm not sure either. The # type: ignore is awful but it'll get things done quickly and run well. I knowassert has been controversial before, so are any of the other alternatives performant?

I added that for now.

einarf · 2023-06-08T23:56:14Z

Still assuming this is a WIP

…into AddMissingTypeHints

pushfoo · 2023-09-27T03:00:54Z

arcade/draw_commands.py

+    for i in range(len(point_list) - 1):
+        point1 = point_list[i]
+        point2 = point_list[i + 1]
+
+        points = get_points_for_thick_line(point1[0], point1[1], point2[0], point2[1], line_width)
+        triangle_point_list.extend([points[1], points[0], points[2], points[3]])
+
+    # Handle the last segment (connecting back to the start)
+    point1 = point_list[-1]
+    point2 = point_list[0]
+    points = get_points_for_thick_line(point1[0], point1[1], point2[0], point2[1], line_width)
+    triangle_point_list.extend([points[1], points[0], points[2], points[3]])


Why is this in a type hint PR?

Do we have unit tests covering these changes?

If this is a needed improvement. If it works, this should be in another, separate PR.

eruvanos · 2023-11-08T21:33:14Z

Can not be merged right now, I changed it to be a draft.

Grant Hur added 2 commits April 22, 2023 20:29

1st version

f9de04d

Add more Type Hints

4cc412c

cspotcode reviewed Apr 24, 2023

View reviewed changes

Grant Hur added 7 commits April 24, 2023 11:25

Fix cspotcode's review

2c4ecc8

Add types to sprite.py

0d166e4

Try to fix some issues

3076ed4

Fixed more github stuff

97cb872

Fix easing.py somewhat

c3c590f

Mics changes bc tests are breaking

ffedb28

.

be086d0

gran4 mentioned this pull request Apr 25, 2023

I screwed up, I meant to do it on https://github.com/pythonarcade/arcade/pull/1720 but I did it on the wrong branch. #1727

Closed

Grant Hur added 12 commits April 25, 2023 14:52

Changes bc git complained

06b6ddd

Fixes problems in 4 files

171af9b

Attempt to fix git again

9623b62

type hint problems in property.py

a432ca6

attempts to fix property.py

5294ea1

I was misunderstanding the test error messages

5b0d1f0

.

211f6a2

Attempt to fix property

3698b0f

.

51a15eb

.

71ef4b2

Check changes in mixins.py

7de2f68

Address no slots in View

7873b18

Fixes pythonarcade#1743

pushfoo suggested changes May 3, 2023

View reviewed changes

Partially added __slots__

f50d5e7

Pyglet doesn't have __slots__ so I didn't know all the variables

Grant Hur added 3 commits May 2, 2023 18:29

Role back __slots__

4b3789d

.

8563913

.

18c7c2c

cspotcode mentioned this pull request May 6, 2023

Address type issues #1751

Merged

Remove generics

5fe6a23

gran4 requested review from pushfoo, eruvanos and cspotcode May 27, 2023 21:53

cspotcode reviewed May 31, 2023

View reviewed changes

arcade/gui/property.py Outdated Show resolved Hide resolved

Grant Yul Hur added 3 commits June 3, 2023 14:28

Requested fixes

7f6de70

correct import in application

a3e51d4

Add # type: ignore to speed it up

a5a6fc7

Line was too long

58805a7

Grant Yul Hur and others added 12 commits June 12, 2023 23:36

Fixes

3e867fa

Merge branch 'pythonarcade:development' into AddMissingTypeHints

971e8f5

..

29e8632

Merge branch 'AddMissingTypeHints' of https://github.com/gran4/arcade …

99595de

…into AddMissingTypeHints

...

6a0a779

..

135f7ca

.

4b344bf

.

aeb3819

.

929d87f

.

eb1f7d7

.

53237fa

.

21f3a8b

gran4 requested a review from cspotcode July 5, 2023 21:06

Update draw_commands.py

bc472c2

pushfoo suggested changes Sep 27, 2023

View reviewed changes

eruvanos marked this pull request as draft November 8, 2023 21:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add missing type hints #1720

Add missing type hints #1720

gran4 commented Apr 24, 2023 •

edited

pushfoo left a comment

gran4 commented May 3, 2023 •

edited

cspotcode commented May 31, 2023

pushfoo commented Jun 2, 2023 •

edited

gran4 commented Jun 3, 2023

einarf commented Jun 8, 2023

pushfoo Sep 27, 2023

eruvanos commented Nov 8, 2023

Add missing type hints #1720

Are you sure you want to change the base?

Add missing type hints #1720

Conversation

gran4 commented Apr 24, 2023 • edited

pushfoo left a comment

Choose a reason for hiding this comment

gran4 commented May 3, 2023 • edited

cspotcode commented May 31, 2023

pushfoo commented Jun 2, 2023 • edited

gran4 commented Jun 3, 2023

einarf commented Jun 8, 2023

pushfoo Sep 27, 2023

Choose a reason for hiding this comment

eruvanos commented Nov 8, 2023

gran4 commented Apr 24, 2023 •

edited

gran4 commented May 3, 2023 •

edited

pushfoo commented Jun 2, 2023 •

edited