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

Introduce namedtuple types for position and size #2388

Closed
freakboy3742 opened this issue Feb 11, 2024 · 4 comments · Fixed by #2585
Closed

Introduce namedtuple types for position and size #2388

freakboy3742 opened this issue Feb 11, 2024 · 4 comments · Fixed by #2585
Labels
enhancement New features, or improvements to existing features. good first issue Is this your first time contributing? This could be a good place to start!

Comments

@freakboy3742
Copy link
Member

freakboy3742 commented Feb 11, 2024

What is the problem or limitation you are having?

Toga makes extensive use of tuples to pass around size and position values (e.g., the size and position of a window)

Tuples work for this purpose, and are extremely convenient from the perspective of the user providing input; but can make usage obscure because they rely on understanding that, for example mysize[0] is width, and mysize[1] is height.

Describe the solution you'd like

Toga should define namedtuples for Size and Position data types. This would allow the usage of mysize.width and mysize.height, myposition.x and myposition.y at runtime.

These type declarations should be in a toga.types package, but exposed through the toga namespace, so toga.Size() and toga.Position() would be legal references.

Any setter or method that accepts a tuple should continue to do so in addition to the new type; however, internally, Toga should cast the "raw" tuple to the named tuple as early as is convenient. Any value returned by a Toga API should be in the namedtuple form.

A SizeT type alias for "Size in nametuple or raw tuple form" should be defined (and similar for position), so that there's a convenient way for inputs to declare that both forms are valid. SizeT would only be used for input; output should always have the type annotation toga.Size.

Any internal code referencing size[0] could then be updated to reference size.width etc; however, this should be considered optional, as the existing size[0] usage should continue to work.

Describe alternatives you've considered

  • Do nothing, and continue to use raw tuples
  • Use a full class, rather than a namedtuple. This essentially gains no benefit over namedtuple, other than needing to define all the extra methods to make the new class behave like a tuple.

Additional context

Care should be taken to ensure that backends return data in the new types, rather than tuples. Adding type checks to the testbed backend may be advisable to ensure that the returned types are always honoured.

@freakboy3742 freakboy3742 added enhancement New features, or improvements to existing features. good first issue Is this your first time contributing? This could be a good place to start! labels Feb 11, 2024
@Dundy703
Copy link

Could I give this a shot?

@freakboy3742
Copy link
Member Author

@Dundy703 Go right ahead! We've got a contribution guide to help step you through the process of setting up a development environment and generating your first PR. If you've got any questions, let us know.

@freakboy3742
Copy link
Member Author

By coincidence, I've needed to introduce a LatLng namedtuple type as part of #2379; that PR hasn't merged yet, but anyone implementing a fix for this issue may find that PR helpful as a guide - most of the infrstructure introduced there will be identical to what is needed by this issue.

@rashenck
Copy link
Contributor

Going to picking this up at the Pycon US sprint!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features, or improvements to existing features. good first issue Is this your first time contributing? This could be a good place to start!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants