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

__match_args__ should work with pathlib.Path #108128

Closed
jgarvin opened this issue Aug 19, 2023 · 7 comments
Closed

__match_args__ should work with pathlib.Path #108128

jgarvin opened this issue Aug 19, 2023 · 7 comments
Labels
stdlib Python modules in the Lib dir topic-pathlib type-feature A feature request or enhancement

Comments

@jgarvin
Copy link

jgarvin commented Aug 19, 2023

Feature or enhancement

Has this already been discussed elsewhere?

I have already discussed this feature proposal on Discourse

Links to previous discussion of this feature:

https://discuss.python.org/t/pattern-matching-and-paths/12819

Not exactly a huge discussion but at least illustrates that more than one person has run into this in practice :)

Proposal:

Ideally this should work:

from pathlib import Path

x = Path("/tmp/hello")

match x:
    case Path(_) as path:
        pass

It's not uncommon to write typed APIs that take str | Path and currently you can't use match to handle this. The discussion includes a snippet that would make it work if adopted, so I suspect this is an easy win.

@jgarvin jgarvin added the type-feature A feature request or enhancement label Aug 19, 2023
@sunmy2019 sunmy2019 added the stdlib Python modules in the Lib dir label Aug 19, 2023
@ronaldoussoren
Copy link
Contributor

Why would having this feature be useful? In my own code I generally either convert to a known type (generally pathlib.Path) when I need to manipulate the value, or just pass it through unchanged. Using match to destructure a Path into a string feels like a code smell to me (from a type with a rich API that represents the kind of object to a plain string).

Also, how would adding __match_args__ interact with the using Path's with match in ways that already work, such as the code below?

some_path: Path = ...

match some_patch:
    case Path(suffix=".png"): 
         ... # process PNG files

    case Path(suffix=".py"):
        ... # process Python source

    case Path(suffix=suffix):
        print(f"Unknown file extension {suffix!r}")

@henryiii
Copy link
Contributor

The correct way to write that is this:

from pathlib import Path

x = Path("/tmp/hello")

match x:
    case Path() as path:
        pass

And that works today. You don't need to put _ inside the parentheses; pattern matching sort of looks like constructors, but it's not, it's unique syntax.

It's only if you need to deconstruct this as a string that it would be useful, which I think there are occasionally use cases for. It's especially useful in the case I listed originally, where if __match_args__ = ("__fspath__()",) was supported (note the parentheses! That's not supported today.) and added to os.PathLike, then you could do:

case str(x) | os.PathLike(x):
    ... # x is a string

But I'm afraid that would then require subclasses to also implement __fspath__, which is a no-go. It's still mildly handy for Path (substitute Path above), and feels like it should work given the other built-ins support this (and I think the main reason it doesn't already support this is because the way to get a string out is via a method, not a property, and pattern matching doesn't support that currently).

Also, how would adding __match_args__ interact with the using Path's

Same way any other one works. For example (using Python 3.12 for Path subclassing):

import pathlib

class Path(pathlib.Path):
   __match_args__ = ("__match_self_prop__",)

   @property
   def __match_self_prop__(self) -> str:
       return self.__fspath__()
>>> f(Path("x.py"))
Got 'x.py'
>>> f(Path("x.txt"))
Didn't match

@AlexWaygood
Copy link
Member

AlexWaygood commented Aug 21, 2023

where if __match_args__ = ("__fspath__()",) was supported (note the parentheses! That's not supported today.)

Yeah, that would be a pretty significant feature request, which would require a PEP @henryiii :)

It sounds pretty similar to the things @msullivan was proposing at the Python Language Summit this year: https://pyfound.blogspot.com/2023/05/the-python-language-summit-2023-pattern.html

@jgarvin
Copy link
Author

jgarvin commented Aug 21, 2023

I didn't know I should use Path() instead of Path(_). The first pattern matching examples I found used ClassName(_) to match on type, so I had just been copying the pattern, which works for my own classes that don't have __match_args__ and for str and set but not Path. I use flake8 and mypy and neither of them suggested the fix.

@henryiii
Copy link
Contributor

X(_) will match a class with one positional argument. While X() will match a class with any number of positional arguments. Since the number of positional arguments depends on the class, not the instance, there's no reason to ever write X(_) that I'm aware of. I expect it was a mistake because someone assumed that this had to match class constructors, which is not true.

@barneygale
Copy link
Contributor

Is there anything to do here? I'm a bit of a newbie to __match_args__ so I don't quite follow all the discussions above.

@jgarvin
Copy link
Author

jgarvin commented May 9, 2024

@barneygale nah the take away is if you are trying to detect the type (what I was doing) to match on Foo() instead of Foo(_). This can be closed.

@jgarvin jgarvin closed this as completed May 9, 2024
@barneygale barneygale closed this as not planned Won't fix, can't repro, duplicate, stale May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib Python modules in the Lib dir topic-pathlib type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

6 participants