Add missing type hints

[gran4]

trafficstars

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.

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

[eruvanos]

#1751 was merged, should we review this one, so it gets merged, too?

[cspotcode]

Definitely, I can help review tonight.

[cspotcode]

Looks like this PR has conflicts with the development branch that will need to be resolved.

[gran4]

Can you explain to me why I should leave generics in a separate PR?

[pushfoo]

Can you explain to me why I should leave generics in a separate PR?

I'd like to get this PR merged sooner rather than later because:

1. Some of these annotations will be very nice to have.
2. It includes changes to some areas I'd like to work on without risking merge conflicts
3. Generics can be tricky to get right

This PR has already been in progress for a month, and I do not want to delay it further by adding more changes to an already-large PR.

[eruvanos]

@gran4 @pushfoo @cspotcode Just mention me, if you need any help or if you thing we are ready for merging.

[cspotcode]

@pushfoo @eruvanos from my perspective, as soon as @pushfoo's feedback is addressed, this PR is ready to merge. I'm sure if anything is wrong, CI will tell us about it soon enough.

Agreed that we should not add anything new to this PR.

[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

[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: https://github.com/pythonarcade/arcade/pull/2143