←back to thread

317 points est | 10 comments | | HN request time: 1.125s | source | bottom
Show context
logicallee ◴[] No.17448729[source]
This is unpythonic, breaking about half of these design rules:

https://en.m.wikipedia.org/wiki/Zen_of_Python

For anyone who hasn't read the original PEP link, what do you suppose this does? Guess its meaning:

1.

    if (row := cursor.fetchone()) is None:
          raise NotFound
      return row
2.

Next guess the meaning of this - what does it look like it does?

  row = cursor.fetchone()
  if row is None:
      raise NotFound
  return row


Answers:

The first does the second. The second doesn't need explanation. I literally don't need to tell you what it does.

(The two examples are from the link.)

Now you might think that this means I'm against this PEP.

But actually [EDIT: because I know there is a VERY high bar to being accepted into Python] to me it means this is going to save an incredible amount of time - it must be very good indeed to be accepted.

So if they decided to adopt it - sure, I'll use it. And I bet it's great in practice.

It's also super explicit. If you don't know what := does you will go look it up.

If Python is still driven by Python design philosophy and all that discussion, then this will be very helpful. It surely had a very high threshold to meet.

replies(2): >>17448868 #>>17449189 #
marcus_holmes ◴[] No.17448868[source]
I'm confused. Are you saying it's unpythonic but you like it anyway?
replies(1): >>17449083 #
1. logicallee ◴[] No.17449083[source]
I've added an edit to my comment to help you through the logical jump, and in the rest of this comment I write it out explicitly.

So, my reasoning is that I surely must end up liking it, if it ended up accepted despite breaking every rule in the book. They wouldn't accept it for something that wasn't incredibly useful. My first impression is that it's incredibly ugly, unpythonic, and unreadable (what does it even do?), and this is pretty "obvious".

So arguing based on what I know about Python design philosophy, and the fact that this has been so deeply discussed, it must be absolutely fantastic.

I wouldn't think it's great (if you gave me the syntax), I'd recite all the Python Values it breaks -

https://en.m.wikipedia.org/wiki/Zen_of_Python :

- Beautiful is better than ugly.

Nope, this is very ugly.

- Simple is better than complex.

Nope, this isn't simple.

- Flat is better than nested.

Nope, this explicitly exists to let you nest something.

- Sparse is better than dense.

Nope, this explicitly exists to make code denser.

- Readability counts.

Nope: this is literally not readable. You don't know what the first code example I gave does. Normally with Python you just "write pseudocode and make sure you indent it properly."

- Special cases aren't special enough to break the rules.

This is a special case.

but here we get to the next rule, which trumps ALL of the above:

Although practicality beats purity.

- There should be one—and preferably only one—obvious way to do it.

This is broken, as now you can do it on two lines or one line.

- If the implementation is hard to explain, it's a bad idea.

It's kind of hard to explain, look at all our comments.

- If the implementation is easy to explain, it may be a good idea.

This isn't the case here.

I will grant that this is extremely explicit syntax. In this sense it is MUCH better than overloading = based on different contexts or something. As I mentioned above, anyone who sees := knows that it's a "known unknown" -- WTF is this?

So we are left with a single solitary Pythonic value:

- Practicality beats purity.

Based on this alone, I reason that it must be great to have been accepted. It must be extremely practical. It must save a lot of time.

I am quite disposed to thinking (honestly!) that it must be pretty darn great. It will save a lot of time.

There's no way that it made it into the language if it didn't, since it's so obviously ugly and unpythonic.

But practicality beats purity. I'm sure I'll love it!

replies(2): >>17450188 #>>17451044 #
2. garettmd ◴[] No.17450188[source]
Just yesterday I was thinking I would like to have something like this. I didn't know that this type of feature had a name, nor did I know what it would/could look like, but now that I see it in this PEP, it looks awesome to me. Yes, it does also look like something that could be greatly abused. But I think the way to temper that is to add guidelines for it (for linting) and to not bring this feature up for learners until they've reached a more advanced level (which I figure will likely happen, as it will be new for most experienced Python coders for a while)
3. abuckenheimer ◴[] No.17451044[source]
I don't know the Kirill Balunov example is pretty beautiful/simple/flat/readable

    if reductor := dispatch_table.get(cls):
        rv = reductor(x)
    elif reductor := getattr(x, "__reduce_ex__", None):
        rv = reductor(4)
    elif reductor := getattr(x, "__reduce__", None):
        rv = reductor()
    else:
        raise Error("un(shallow)copyable object of type %s" % cls)
especially when you compare it to the existing implementation:

    reductor = dispatch_table.get(cls)
    if reductor:
        rv = reductor(x)
    else:
        reductor = getattr(x, "__reduce_ex__", None)
        if reductor:
            rv = reductor(4)
        else:
            reductor = getattr(x, "__reduce__", None)
            if reductor:
                rv = reductor()
            else:
                raise Error("un(shallow)copyable object of type %s" % cls)
replies(3): >>17451765 #>>17452145 #>>17460009 #
4. carapace ◴[] No.17451765[source]
Nah, write a little gadget:

    def f(F, *args):
      f.reductor = F(*args)
      return bool(f.reductor)

    if f(dispatch_table.get, cls)):
        rv = f.reductor(x)
    elif f(getattr, x, "__reduce_ex__", None):
        rv = f.reductor(4)
    elif f(getattr, x, "__reduce__", None):
        rv = f.reductor()
    else:
        raise Error("un(shallow)copyable object of type %s" % cls)

Same pattern works for re.match objects.

Once you abstract the patterns you can drive towards data-driven code:

    K = (
      (dispatch_table.get, (cls,), (x,)),
      (getattr, (x, "__reduce_ex__", None), (4,)),
      (getattr, (x, "__reduce__", None), ()),
    )

    for F, a, b in K:
      reductor = F(*a)
      if reductor:
        rv = reductor(*b)
        break
    else:
        raise Error("un(shallow)copyable object of type %s" % cls)

Syntax is the enemy, never substitute syntax for thought.
replies(1): >>17452735 #
5. HelloNurse ◴[] No.17452145[source]
The new version is a little shorter (not important), without "technical" lines with reductor assignments and with the important getattr() expressions on the if lines (subtle and subjective, but somewhat important), and consistently indented with similar things at the same level (quite important).

All this for the small cost of learning a few simple scoping rules and the negligible cost of occasionally discovering the special cases in which the new syntax cannot be used.

6. joshuamorton ◴[] No.17452735{3}[source]
Your second example is great except that any error messages that get raised from inside getattr are going to be inscrutable. The traceback will be uninformative, and you'll be forced to drop into a debugger to figure out what's going on.

At runtime, your second example is actually significantly less explicit because so much is hidden away in mutable state.

replies(1): >>17453466 #
7. carapace ◴[] No.17453466{4}[source]
> Your second example is great except that any error messages that get raised from inside getattr are going to be inscrutable. The traceback will be uninformative, and you'll be forced to drop into a debugger to figure out what's going on.

I started to agree with that but then I realized: I don't think you can get getattr to raise an error here at all. The keys are strings, the default value is provided, I don't think there's a fault path here.

In general though, I totally agree with you. I use less-cute code shamelessly when it will aid debugging. I write for the debugger. ;-)

> At runtime, your second example is actually significantly less explicit because so much is hidden away in mutable state.

Yeah, I see what you mean. You don't know which "F" or "reductor" triggered the error because it doesn't show up in the traceback because each assignment doesn't have its own line in the code. That's a good point. I've been there, and it sucks.

In this specific case I would actually most likely write out the three if.. else.. chain. If there were four or more I would use the gadget style. It's right on the borderline, and I would suspect that there won't be a fourth option in the future (although usually that would be a bad assumption; in this case I speculate the the object model isn't going to change any time soon.)

replies(1): >>17454003 #
8. joshuamorton ◴[] No.17454003{5}[source]
>I started to agree with that but then I realized: I don't think you can get getattr to raise an error here at all. The keys are strings, the default value is provided, I don't think there's a fault path here.

Of course you can ;) __getattr__ could invoke a network request that fails for all you know.

But yes this was more a general comment on this layout. The data-driven layout, while very explicit in code, is actually awful when you encounter issues at runtime, for exactly the reasons you describe.

While I'm not a fan of this PEP's syntax, I will say that I do think it someone helps here, it reduces the boilerplate from 3-4 lines in some cases to a single line, which in practice makes this more visually declarative, and keeps tracebacks sane.

That said this pattern is rare enough for me that I don't think I'll be using this tool anytime soon.

replies(1): >>17454820 #
9. carapace ◴[] No.17454820{6}[source]
> Of course you can ;) __getattr__ could invoke a network request that fails for all you know.

OMG! Yes, of course, but at that point your traceback has bigger problems. :-)

10. SmirkingRevenge ◴[] No.17460009[source]
This is the kind of thing I might turn into a simple function with short circuiting returns. I suppose the new syntax is just as readable, but I kind of like it this way a bit better.

    def get_reductor_value(cls, x):
        reductor = dispatch_table.get(cls)
        if reductor:
            return reductor(x)

        reductor = getattr(x, "__reduce_ex__", None)
        if reductor:
            return reductor(4)

        reductor = getattr(x, "__reduce__", None)
        if reductor:
            return reductor()

        raise Error("un(shallow)copyable object of type %s" % cls)