Skip to content

gh-106246: Allow the use of unions as match patterns - #118525

Closed
tmke8 wants to merge 2 commits into
python:mainfrom
tmke8:fix-issue-118524
Closed

gh-106246: Allow the use of unions as match patterns#118525
tmke8 wants to merge 2 commits into
python:mainfrom
tmke8:fix-issue-118524

Conversation

@tmke8

@tmke8 tmke8 commented May 2, 2024

Copy link
Copy Markdown
Contributor

This didn't require many code changes. Though I'm not entirely sure about code style and whether I added enough tests.

@ghost

ghost commented May 2, 2024

Copy link
Copy Markdown

All commit authors signed the Contributor License Agreement.
CLA signed

Comment thread Lib/test/test_patma.py
w = 0
self.assertIsNone(w)

def test_union_type_postional_subpattern(self):

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

english is not my mother tongue, but postional must not to be replaced by positional?

Comment thread Lib/test/test_patma.py
import dataclasses
import enum
import inspect
from re import I

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

accidental?

Comment thread Lib/test/test_patma.py
x = 0
match x:
case IntOrStr():
x = 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Add a test that involves a union that does not match.

@nineteendo

Copy link
Copy Markdown
Contributor

Please rename the title to use gh-106246.

Comment thread Python/ceval.c
return NULL;
}
// Subpatterns are not supported for union types:
if (_PyUnion_Check(type)) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I don't like the fact that we call _PyUnion_Check(type) twice for no real reason.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we save the result in a variable? Or would it cleaner to handle unions first?

@randolf-scholz

randolf-scholz commented May 3, 2024

Copy link
Copy Markdown
Contributor

Is there any particular reason for disallowing subpatterns? The case for matching a union in the first place is typically because they share some relevant interface or attribute.

For example:

FuncDef = ast.FunctionDef | ast.AsyncFunctionDef

match node:
    case FuncDef(name=name, body=body):
        ...

instead of

match node:
    case ast.FunctionDef(name=name, body=body) | ast.AsyncFunctionDef(name=name, body=body):
        ...

@carljm carljm changed the title gh-118524: Allow the use of unions as match patterns gh-106246: Allow the use of unions as match patterns May 3, 2024
@randolf-scholz

Copy link
Copy Markdown
Contributor

@sobolevn @JelleZijlstra @sylvainmouquet @tmke8 I pushed a variant of this pull request that addresses the raised issues and also support subpatterns #118644.

@tmke8 tmke8 closed this May 7, 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.

6 participants