Yeah, and what I was saying is that the base probability of this kind of typo is higher than the base probability of any kind of typo due to the fact that 1) a comma is a part of data structure syntax; 2) refactoring code that has used a comma is extremely common.
1 Like
Irrelevant; what you need to show is that the probability of this kind of typo is higher - and not just marginally, but VASTLY higher - than the probability that the code was written correctly.
“Need”? Why? Because you say so?
2 Likes
Agreed. We even put in a check for this specific case in iommi, and it has saved us lots of debugging time over the years.
1 Like
It always amuses me when people think that it’s our job to argue their points for them.
No, not because “we say so”, but because:
- The status quo wins a stalemate.
- By default, we err on keeping backwards-compatibility rather than breaking it.
3 Likes
That… made no sense.
Steven elaborated on the sentiment behind my post. The fact is, you don’t “need” to because I say so, or because someone else says so, or anything; you only “need” to because YOU care about this. If everyone else in the world decides that this entire topic is irrelevant, nothing will happen, the status quo will remain, and you are the only one who feels strongly about it. In order to get anything to change, you have to convince people. Hence, the need comes from you, not us.
That seems disingenuous at best. And also unrelated to your statement that I want you to make my point. I’ve made my point, and several people have concurred. I have produced some limited evidence. I don’t get where you’re coming from claiming I want you to argue my point. That’s simply not true.
In any case, I don’t see why you’re derailing this discussion with this line of “argument”…
1 Like
It’s not derailing the argument; I’m answering your question about next steps. And I’m done here; you can go ahead and not convince people, it doesn’t bother me at all.
The flake8-tuple plug-in catches this one.
There’s a nice example of that in test_functools.py. TestLRUPy.cached_func works exactly like this.
The author was not a beginner, far from it. The spurious trailing comma trips us all up from time to time.
It is not a spurious trailing comma. It is a clever trick to work around the fact that function attribute of a class becomes a method.
3 Likes
We’re talking about cpython/test_functools.py at main · python/cpython · GitHub I assume? Wouldn’t that have been better written as cached_func = static_method(py_cached_func)?
In any case, I think it’s clear that it’s not clear it was on purpose 
A comment would have been good at least.
Agreed a comment would have been nice. But adding parentheses wouldn’t have changed that fact.
3 Likes
Your argument is that these expert coders typoed the comma and then instead of removing it they added in a completely spurious subscript to work around it. Really? That doesn’t seem very plausible to me.
You should consider Chesterton’s Fence before concluding that the experienced and competent authors who wrote that code didn’t know what they were doing. When experienced coders write something that looks weird, and it passes code review from other experienced coders, there is probably a good reason for it.
In the code you are looking at, cls.cached_func is required to be a plain ordinary function not a method. That module dates back to Python 2. In Python 2, without the comma, cls.cached_func will return an unbound method, not a function.
The easiest way to prevent that is to put the function in a tuple.
I’m reasonably sure, say, 90% sure, that this is now obsolete. In Python 3, lookups of functions on the cls no longer return an unbound method, they return the function object without a method wrapper. So we could very likely safely remove the comma and the subscript.
But it almost certainly was intentional back in the day, not a mistake.
So I would say that this is a nice example, not of the bug you thought it was, but of Chesterton’s Fence. In this case we can probably take the fence down, but it once was needed.
5 Likes
Really? If you remove the comma and the [0], the test still passes.
Even experts can become confused and go for the quick fix, although I concede that it may not have been what happened here. I did test the theory before posting by removing commas and [0]'s and running the test, which succeded; in that light it seemed very plausible.
Hint: also check the coverage output. Spoiler alert: it differs.
You can go even further than removing that little bit. If you remove all code from the tests (replace the function bodies with just pass), they probably also still pass.