Skip to content
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

Update section __init__ type hints #2527

Merged
merged 2 commits into from
Feb 4, 2025

Conversation

elegantiron
Copy link
Contributor

Update left, bottom, width, and height to hint that they accept int or float

fixes #2525

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Update left, bottom, width, and height to hint that they accept int or float
@elegantiron
Copy link
Contributor Author

working on the failed checks

@einarf
Copy link
Member

einarf commented Feb 2, 2025

We do have AsFloat that would likely be more specific in this case?

AsFloat = Union[float, int]

@elegantiron
Copy link
Contributor Author

Looking at the docs for type hinting:

# On Python 3.10+, use the | operator when something could be one of a few types
x: list[int | str] = [3, 5, "test", "fun"]  # Python 3.10+
# On earlier versions, use Union
x: list[Union[int, str]] = [3, 5, "test", "fun"]

We should either use Union or we should drop support for Python 3.9.

I assume dropping support for Python 3.9 is going to be significantly more involved than using Union to stay compatible and people will prefer using Union over changing the supported versions.

@einarf
Copy link
Member

einarf commented Feb 4, 2025

We should either use Union or we should drop support for Python 3.9.

I think from __future__ import annotations backports everything even for 3.9.

@pushfoo
Copy link
Member

pushfoo commented Feb 4, 2025

We should either use Union or we should drop support for Python 3.9.

TL;DR: Soon (in a few months)

For the moment switching back to Union would be counter-productive:

  1. It's eight months until Python 3.9 EOLs in October of 2025
  2. We've been incrementally switching to | in anticipation of that

As a bonus, the doc cited is mypy doc. They have ideas which clash with our codebase(s). Specifically, we face the same issues as pyglet does with mypy due to Rect being a NamedTuple (from the pyglet pyproject.toml):

[[tool.mypy.overrides]]
# mypy incorrectly reports issues everywhere in pyglet.math because:
# 1. pyglet.math is almost entirely typing.NamedTuple subclasses
# 2. The mypy project has the following long-standing stance on
#    supporting collections.namedtuple and typing.NamedTuple:
#    "Duplicate of #5613, still low priority, better use dataclasses,
#    as suggested above."
#    https://github.com/python/mypy/issues/5944#issuecomment-441285456
module = "pyglet.math"
ignore_errors = true

@pushfoo
Copy link
Member

pushfoo commented Feb 4, 2025

We do have AsFloat that would likely be more specific in this case?

I think it was lazy shorthand / a way to make pyright pass in specific cases.

Copy link
Member

@pushfoo pushfoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my eyes, this has better legiblity than AsFloat. The tab-completion ergonomics aren't as good, but 🤷

@pushfoo pushfoo merged commit 23df106 into pythonarcade:development Feb 4, 2025
8 checks passed
@elegantiron elegantiron deleted the patch-2 branch February 4, 2025 12:04
pushfoo pushed a commit to pushfoo/arcade that referenced this pull request Feb 12, 2025
* Update section __init__ type hints

* Update left, bottom, width, and height to hint that they accept int or float

* fixes for the automated checks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

arcade.View hints the wrong type for height and width
3 participants