Skip to content

Conversation

@radarhere
Copy link
Member

No description provided.

Comment on lines 78 to 81
bbox: tuple[tuple[int, int], tuple[int, int]]
| list[tuple[int, int]]
| list[int]
| tuple[int, int, int, int],
Copy link
Member

Choose a reason for hiding this comment

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

This monster would be a good one to define as a type alias in _typing.py.

Copy link
Contributor

@Yay295 Yay295 Jan 31, 2024

Choose a reason for hiding this comment

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

Would tuple[list[int], list[int]] also be a valid option in this location?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've pushed a commit to just simplify it down to Sequence[int | Sequence[int]] instead.

Copy link
Member

Choose a reason for hiding this comment

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

Would that also be useful as an alias, for clearer re-use?

return bool


CoordList = Union[Sequence[float], Sequence[Sequence[float]]]
Copy link
Member

Choose a reason for hiding this comment

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

Maybe just Coords? It's shorter, and a sequence can be a not-a-list such as a tuple:

Suggested change
CoordList = Union[Sequence[float], Sequence[Sequence[float]]]
Coords = Union[Sequence[float], Sequence[Sequence[float]]]

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure, I've made this change.

@hugovk hugovk merged commit 811dd15 into python-pillow:main Feb 6, 2024
@hugovk
Copy link
Member

hugovk commented Feb 6, 2024

Thanks!

@radarhere radarhere deleted the type_hints branch February 6, 2024 23:17
@radarhere radarhere mentioned this pull request Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants