Add missing type hints

[gran4]

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.

[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.

[gran4]

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.

[cspotcode]

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]

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:

1. This is an internal class which only has a single current use: the particle emitter
2. 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]

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]

Still assuming this is a WIP

[pushfoo]

1. Why is this in a type hint PR?
2. 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]

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

[einarf]

Thanks for the work on this, but this is sadly not possible to merge at this time.

I manually salvaged and merged this branch in: #2143