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

pyAutoGui: Correct Return Type of position() to Match Actual Behavior #11267

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

codekoriko
Copy link

@codekoriko codekoriko commented Jan 10, 2024

Point being a Tuple of float it does not match the actual behavior of the position() function:

def position(x=None, y=None):
    """
    Returns the current xy coordinates of the mouse cursor as a two-integer tuple.

    Args:
      x (int, None, optional) - If not None, this argument overrides the x in
        the return value.
      y (int, None, optional) - If not None, this argument overrides the y in
        the return value.

    Returns:
      (x, y) tuple of the current xy coordinates of the mouse cursor.

    NOTE: The position() function doesn't check for failsafe.
    """
    posx, posy = platformModule._position()
    posx = int(posx)
    posy = int(posy)
    if x is not None:  # If set, the x parameter overrides the return value.
        posx = int(x)
    if y is not None:  # If set, the y parameter overrides the return value.
        posy = int(y)
    return Point(posx, posy)

@AlexWaygood AlexWaygood changed the title Correct Return Type of position() to Match Actual Behavior pyAutoGui: Correct Return Type of position() to Match Actual Behavior Jan 10, 2024
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Copy link
Sponsor Collaborator

@Avasam Avasam left a comment

Choose a reason for hiding this comment

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

My concern is that this will cause additional casting being needed when a Point is expected and you loose property access:

from math import hypot

def distance_from_origin(point: Point) -> float:
    return hypot(point.x, point.y)

int_point = position(5, 10)

distance = distance_from_origin(int_point)  # (variable) int_point: tuple[int, int]
# Argument of type "tuple[int, int]" cannot be assigned to parameter "point" of type "Point" in function "distance_from_origin"
#  "tuple[int, int]" is incompatible with "Point" Pylance(reportGeneralTypeIssues)

# Argument 1 to "distance_from_origin" has incompatible type "Tuple[int, int]"; expected "Point" Mypy(arg-type)

I think that by using a covariant Generic this can be solved:

from typing import Generic, NamedTuple, TypeVar

_T = TypeVar("_T", int, float, default=float, covariant=True)

class Point(NamedTuple, Generic[_T]):
    x: _T
    y: _T

def position(x: int | None = None, y: int | None = None) -> Point[int]: ...

###

# Downstream code example

from math import hypot

def distance_from_origin(point: Point):  # Same as Point[float]
    return hypot(point.x, point.y)

int_point = position(5, 10)

distance = distance_from_origin(int_point)

This makes possible to pass a Point[int] to a method expecting a Point[float]. And still disallows passing a Point[float] to a method expecting a Point[int].

Once implemented, we could probably improve other methods that are also known to always return a Point[int]. I vaguely remember screen positions being a float only on a single platform (linux?).

The use of default reduces required changes (especially downstream), given this is an edge case. I'm not sure what's the current state of https://peps.python.org/pep-0696/ support. But typeshed's tests should fail if a supported type-checker is not ready for TypeVar defaults.

(CC @AlexWaygood in case you see any issue with my proposal, but it seems sound to me and my quick testing)

@AlexWaygood
Copy link
Member

CC @AlexWaygood in case you see any issue with my proposal, but it seems sound to me and my quick testing

(I'm super busy this week unfortunately, so probably won't be able to take a proper look at this until after the 20th, but I trust your judgement!)

@JelleZijlstra
Copy link
Member

I don't think we're quite ready for PEP 696 in typeshed. Until then, the current annotation is probably best; int is a subtype of float in the type system, after all.

@Avasam Avasam added the status: deferred Issue or PR deferred until some precondition is fixed label Mar 7, 2024
@srittau srittau removed the status: deferred Issue or PR deferred until some precondition is fixed label Mar 20, 2024
@srittau
Copy link
Collaborator

srittau commented Mar 20, 2024

TypeVar defaults are now available in typeshed.

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.

None yet

5 participants