patma icon indicating copy to clipboard operation
patma copied to clipboard

Please document the design decisions behind PEP 634.

Open markshannon opened this issue 5 years ago • 7 comments

The acceptance notice for PEP 634 says:

We consider that the presence of such high-quality documentation must be present on the first release of Python 3.10, and therefore its absence should be considered a release blocker. We do not consider the PEPs or any possible external documentation to be sufficient.

Presumably that mainly includes tutorial and specifications, but I think it would be valuable for maintainers (e.g. me) and users to better understand why things are the way they are.

It might also prevent me, and others, having to rehash discussions you've already had, as we could know the thinking behind various decisions.

markshannon avatar Mar 22 '21 11:03 markshannon

One particular thing I would like to know is whether you considered using __match_args__ to constrict the legal keywords in a class match, so that:

class C:
    __match_args__ = ("a", "b")

match x:
     case C(x=x):
         ...

would raise an exception rather than just failing to match.

markshannon avatar Mar 22 '21 11:03 markshannon

The __match_args__ is actually used for positional arguments, i.e. the pattern C(x=x) has nothing to do with __match_args__. From PEP 634 (highlights by me):

Positional patterns are converted to keyword patterns using the __match_args__ attribute on the class [...]

Hence, no, this would not raise an exception but probably just fail to match—given the rather weird code with C obviously only a stub and lots of xs around I am hesitant about interpreting what exactly you mean with it...

Tobias-Kohn avatar Mar 22 '21 19:03 Tobias-Kohn

And on the question about whether we considered tying __match_args__ to constrain the legal keywords, I think we thought about it and explicitly chose not to link the two -- __match_args__ exists to make positional arguments in patterns like Point(x, y, z) possible without introspection on __init__(). For a class that has attributes which have no direct representation in the positional constructor args, those attributes would just be a distraction in __match_args__ since they shouldn't be usable in a positional role.

Take for example

class Rect:
    def __init__(self, width, height):
        self.width = width
        self.height = height
        self.area = width*height
    __match_args__ = ('width', 'height')

It would not make sense to have area as a positional match arg, but one should be able to write

case Rect(area=a):
    print("Rect with area", a)

Now, I understand that it would be nice if we could constrain the legal attributes, but that should not be rolled into __match_args__.

gvanrossum avatar Mar 22 '21 19:03 gvanrossum

I feel like this is already getting a bit off-topic. 🙂

I think it would be valuable for maintainers (e.g. me) and users to better understand why things are the way they are... It might also prevent me, and others, having to rehash discussions you've already had, as we could know the thinking behind various decisions.

The way I see it, that's what the issue tracker in this repo is for. It's publicly available, searchable, and pretty well-labeled. A lot of the issues also contain links back to relevant mailing list discussions, etc. Not everything is here (or easily-discoverable), but new issues can be opened for anything that's missing.

I'm not sure there's really a better format for documenting for posterity the thought process behind every decision that was made in a huge year-long project like this (above and beyond what PEP 635 already provides)... especially one that is still on-going, to some degree.

brandtbucher avatar Mar 22 '21 20:03 brandtbucher

I understand that you can't cover every decision, but an overview which documents some of the key decisions would be very valuable, IMO.

That's why I asked explicitly about the __match_args__ thing. It's not an obvious thing to document, but I thought it was worth mentioning.

markshannon avatar Mar 23 '21 11:03 markshannon

I understand that you can't cover every decision, but an overview which documents some of the key decisions would be very valuable, IMO.

We thought we did a decent job with PEP 634 and 635 together. Your question about __match_args__ sounded like you were trying to argue with that part of the design, so forgive us for sounding a bit annoyed. Hopefully it's now clear why it is the way it is.

gvanrossum avatar Mar 23 '21 14:03 gvanrossum

I understand that you can't cover every decision, but an overview which documents some of the key decisions would be very valuable, IMO.

Well, that's what PEP 635 is supposed to be.

I'm open to adding "missing" sections to PEP 635 if they are deemed important enough. But I'd rather not get carried away, since we'd probably just be summarizing conversations found in this repo or on mailing lists.

brandtbucher avatar Mar 23 '21 17:03 brandtbucher