More conversions to pattern matching by HalfWhitt · Pull Request #4443 · beeware/toga

HalfWhitt · 2026-05-31T22:54:23Z

I decided to give my brain a break from thinking about substantive changes, and do some sprucing up. None of these match statements make particularly involved use of pattern-matching; they're all basically switch statements. I believe most should be self-explanatory, and hopefully read better; I'll drop some notes inline on oddities.

PR Checklist:

I will abide by the BeeWare Code of Conduct
I have read and have followed the CONTRIBUTING.md file
This PR was generated or assisted using an AI tool

HalfWhitt · 2026-05-31T23:01:31Z

+                    found = all(
+                        getattr(item, attr) == value for attr, value in data.items()
+                    )
+                case Iterable() if not isinstance(data, str):


I know isinstance(data, Iterable) is not, strictly speaking, entirely the same thing as hasattr(data, "__iter__"). But it makes the intent easier to read. If we're particularly worried about, say, performance with runtime-checkable user classes, I can revert this (and the other places like it).

I don't think there was a good reason for this to be an __iter__ check rather than an Iterable check - so this looks good to me.

The "type() if not..." syntax is a new one for me though... not sure how I feel about that... but if that's how match statements work, 🤷

It's called a guard; you can put any conditional you want after a pattern, and it'll be checked if the pattern matches. It even has access to variables bound in the pattern, so you could do something fancy like:

match some_string.split(): case first_word, "and", second_word if first_word == second_word:

Ideally though, I'd REALLY like it if there were just a NonStringIterable ABC to test against (and a NonStringSequence). Strings getting missed when one assumes a non-string sequence/iterable is one of my pet peeves.

Side note: interestingly, strings (and bytes) are already special-cased... by pattern matching.

first, second, third = "abc" # Unpacks just fine, of course match "abc": case first, second, third: # Doesn't match! ...

Which is a good thing in my book, since you'd almost never want that on purpose.

HalfWhitt · 2026-05-31T23:02:53Z


    @value.setter
-    def value(self, value: object) -> None:
+    def value(self, value: datetime.date | str | None) -> None:


I'm not sure if there might have been a specific reason for this object type hint, and its counterpart in TimeInput. I've updated it to include everything the setter can handle. (datetime isn't explicitly listed because it's a subclass of date.)

Like all good typing questions in this repo, most of them go back to the monstrosity of #2252 🤣

There's a few reasons it is typed as object.

It is typed as object to allow _convert_date() to perform the type narrowing and redefine value as a datetime.date

If this setter is updated to use specific types, then I'd probably argue it should be an abstracted type that is used any where a un-normalized date is accepted....but it probably doesn't matter that much

There was a larger issue with typing property methods and their setters in Python

I dont remember all of the implications but Python had some serious issues with method properties that accepted different sets of types; see What to do about setters of a different type than their property? python/mypy#3004

But with Support properties with setter type different from getter type python/mypy#18510, things might be different now

Hm... should I revert it, then?

eh....i think i'm mostly indifferent. your change is probably better from a user perspective since they would see some real types instead of object (however, i think that mostly depends on how python/mypy#18510 was implemented).

Also, I saw this in Python discourse today....interesting timing: https://discuss.python.org/t/class-date-is-unsafe-in-typed-python/107590

So, I wonder if treating date and datetime the same is actually safe here

Oh, okay. So maybe just add in datetime?

That's probably unnecessary; but I did find it interesting that comparisons between date and datetime can lead to unexpected results. So, since the Date widget returns a date, users could experience weird outcomes if they are comparing it to a datetime. It might be total overkill for us to try to handle anything here given this is general Python behavior....but useful nugget to keep in mind.

HalfWhitt · 2026-05-31T23:05:05Z

        """
        close_window = True
-        if self.app.main_window == self:
+        if self.app.main_window is self:


Noticed this == comparison, that I'm pretty sure should be is.

Hmm... there's no defined equality comparsion on Window, so they should be equivalent. But is reads smoother.

Yeah, functionally it's identical. Just a semantic quibble.

johnzhou721

Since we're already doing minor cleanups, if you're intertested you might as well rewrap the following places that you've also touched (granted, I wrote 3/4 of these comments, so it's mostly my fault that I didn't wrap them when I wrote it.)

HalfWhitt · 2026-06-01T19:12:16Z

These lines have been commented out for years:

toga/gtk/src/toga_gtk/widgets/canvas.py

Lines 464 to 476 in 0b9f955

    
           def rehint(self): 
        
               # print( 
        
               #     "REHINT", 
        
               #     self, 
        
               #     self.native.get_preferred_width(), 
        
               #     self.native.get_preferred_height(), 
        
               # ) 
        
               # width = self.native.get_allocation().width 
        
               # height = self.native.get_allocation().height 
        
               width = self.interface._MIN_WIDTH 
        
               height = self.interface._MIN_HEIGHT 
        
               self.interface.intrinsic.height = at_least(width) 
        
               self.interface.intrinsic.width = at_least(height)

@freakboy3742, looks like you were the last one to touch these. Are they fine to remove? If not, I can revert a7c5652.

johnzhou721 · 2026-06-01T19:23:41Z

@HalfWhitt May I ask what tool you use to rewrap comments, or do you just do it manually? Is there a way to show the limit for the number of characters? I'm asking this because I feel slightly guilty as it seems like I'm the person who leaves the most number of unwrapped comments in the codebase...

HalfWhitt · 2026-06-01T23:36:39Z

@HalfWhitt May I ask what tool you use to rewrap comments, or do you just do it manually? Is there a way to show the limit for the number of characters?

I code in Sublime Text, and it has options for placing one or more visual rulers (the vertical grey line on the right), as well as auto-wrapping comments to a specified width. There's even a keyboard shortcut for it.

screenshot of Sublime Text showing ruler

johnzhou721 · 2026-06-02T00:05:07Z

Thanks for the extra tip! I'll try to see if something is possible in vscode... feeling really guilty for not configuring my environment correctly now.

HalfWhitt · 2026-06-02T02:15:19Z

It's really not a big deal. If you can get it set up to do it, that's great. But perfect comment wrapping isn't something worth agonizing over.

freakboy3742

A lot of code churn, but I think I agree these are all net gains in readability. Nice work!

freakboy3742 · 2026-06-04T05:47:48Z

+                decor_view.setSystemUiVisibility(0)
+                self.show_actionbar(True)
+                self._in_presentation_mode = False
+                self.set_window_state(state)


This is an example match statements are (arguably) not as good as if statements - the commonality of the set_window_state() call has been lost in these last two calls.

On net, I think the overall clarity improvement of the "two states match" overrides that downside - but it's worth keeping an eye on the general pattern.

freakboy3742 · 2026-06-04T05:53:19Z

+                    found = all(
+                        getattr(item, attr) == value for attr, value in data.items()
+                    )
+                case Iterable() if not isinstance(data, str):


I don't think there was a good reason for this to be an __iter__ check rather than an Iterable check - so this looks good to me.

The "type() if not..." syntax is a new one for me though... not sure how I feel about that... but if that's how match statements work, 🤷

HalfWhitt added 5 commits May 30, 2026 01:23

Convert command definitions

2770b57

Various 'elif isinstance' conversions

f952a89

Other pattern-matching

dd631ae

Other logic cleanups

3800840

Add change note

d69f442

HalfWhitt commented May 31, 2026

View reviewed changes

HalfWhitt marked this pull request as ready for review May 31, 2026 23:12

johnzhou721 reviewed Jun 1, 2026

View reviewed changes

Comment thread cocoa/src/toga_cocoa/widgets/dateinput.py Outdated

Comment thread cocoa/src/toga_cocoa/widgets/timeinput.py Outdated

Comment thread dummy/src/toga_dummy/command.py Outdated

Comment thread gtk/src/toga_gtk/libs/gtk.py

HalfWhitt added 2 commits June 1, 2026 15:08

Rewrap some comments

4c8b6d8

Remove commented-out code?

a7c5652

More comment wrapping

4e2ca15

freakboy3742 approved these changes Jun 4, 2026

View reviewed changes

freakboy3742 merged commit 7ecb75c into beeware:main Jun 4, 2026
64 checks passed

HalfWhitt deleted the more-pattern-matching branch June 4, 2026 14:05

johnzhou721 pushed a commit to johnzhou721/toga that referenced this pull request Jun 5, 2026

More conversions to pattern matching (beeware#4443)

474aa0e

HalfWhitt mentioned this pull request Jun 15, 2026

Canvas: deprecate write_text, add fill_text & stroke_text #4432

Merged

3 tasks

Uh oh!

Uh oh!

Conversation

HalfWhitt commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist:

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HalfWhitt Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

johnzhou721 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

HalfWhitt commented Jun 1, 2026

Uh oh!

johnzhou721 commented Jun 1, 2026

Uh oh!

HalfWhitt commented Jun 1, 2026

Uh oh!

johnzhou721 commented Jun 2, 2026

Uh oh!

HalfWhitt commented Jun 2, 2026

Uh oh!

freakboy3742 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

HalfWhitt commented May 31, 2026 •

edited

Loading

HalfWhitt Jun 4, 2026 •

edited

Loading