5 ms·

I literally just finished a leetcode problem[1] to practice some python. I found two solutions, one seems more "pythonic" to me and the other seems more clear

I literally just finished a leetcode problem[1] to practice some python. I found two solutions, one seems more "pythonic" to me and the other seems more clear though also more verbose. Clear/Verbose - def maxWidthOfVerticalArea(self, points: List[List[int]]) -> int: x_coords = sorted(x for x, _ in points) greatest_diff_so_far = 0 for i in range(len(x_coords)-1): diff = x_coords[i+1] - x_coords[i] if diff > greatest_diff_so_far: greatest_diff_so_far = diff return greatest_diff_so_far Pythonic - def maxWidthOfVerticalArea(self, points: List[List[int]]) -> int: xs = sorted(x for x, _ in points) return max(x2-x1 for x1, x2 in zip(xs, xs[1:])) I'm curious which approach is bad/worse? On the one hand, the "pythonic" approach seems brief and kind of neat and I feel clever writing it. On the other hand, it is also ~20% slower per my testing, it's less obvious what it's doing, and it would be harder to make some change to the logic (or add logging/metrics) without completely rewriting it. On the one hand, just from the reasons I wrote above, it seems to me like the first way is better and the second worse, but on the other hand, the first way is what I would write while knowing zero python features. Curious if anyone else has thoughts on this. 1 - https://leetcode.com/problems/widest-vertical-area-between-two-points-containing-no-points/submissions/ https://leetcode.com/problems/widest-vertical-area-between-t...

panda88888 6y ago
I would guess the Pythonic approach is slower due to the slicing operation, which creates a new shallow copy of the original array.
squaresmile 6y ago
Interesting, I'm using this [0] to test and got different results. The Pythonic function is 2% slower on 3.9.1 AMD 4900HS but 17% faster on 3.9.1 E5-2660 v2 VPS and 6% faster on 3.6.9 i5-3570S bare metal. I personally think either function is fine. The max finding code pattern in the long function should be familiar to most. For the Pythonic function, I don't think it's less obvious enough to matter. You have a list of stuffs and you want to find the max so you wrap it in the max function. Maybe zip(xs, xs[1:]) can take a bit to recognize but I think it's eh. For the Pythonic one, one can also write all_diffs = [x2-x1 for x1, x2 in zip(xs, xs[1:])] return max(all_diffs) Using list is also markedly faster than the Pythonic generator for me, which is to be expected: ~16% faster than both. [0] https://gist.github.com/squaresmile/8dd08898851a4e19359cdb30e0f7b843 https://gist.github.com/squaresmile/8dd08898851a4e19359cdb30...
- ALittleLight 6y ago
  That is interesting. Trying your timeit code I've come to see that maybe something was wrong about how I was estimating the different performance. I was just using cProfile and profiling the two functions with big inputs and then comparing the cumulative time results that way. When I use your timeit examples my results (Intel Core i7-6700K CPU @ 4.00GHz) are different and closer to yours (pythonic ~1% slower). Thanks for sharing.
travisjungroth 6y ago
I just solved this problem yesterday and my code looked exactly like your second one. Even xs was the same, except I used a and b instead of x1 and x2 cause I always do that. But I actually like yours more. This is a very personal opinion, but I do like the second one more. I find it easier to read, but maybe that's just because I'm used to that style. And maybe you're more likely to have to rewrite it for a change, but that's kind of okay since there's less code there. On readability, here's how I'd read it out loud: Sort all the Xes from points. Return the max of the difference between two points that are next to each other. I find that quicker to put in my head than the equivalent parsing of the first solution, and exactly the intention of the code.
floxy 6y ago
FWIW, it was easier to me to follow your intent with the second example. The only clunky thing about is was the zip followed by the comprehension. It might be slightly nicer with something like: return max(xs[1:]-xs[:-1]) ...using something like numpy. I wonder if APL/J has successive element operator.
- adamjb 6y ago
  If you're gonna bring numpy into it you may as well use np.diff(): def maxWidthOfVerticalArea(self, points: List[List[int]]) -> int: return np.diff(sorted(x for x, _ in points)).max()
- ahgamut 6y ago
  J has the verbs behead[0] (for xs[1:]) and curtail[1] (for xs[:-1]), so the J oneliner for the above code would be <./ ([&}. - }:) xs [0] https://code.jsoftware.com/wiki/Vocabulary/curlyrtdot https://code.jsoftware.com/wiki/Vocabulary/curlyrtdot [1] https://code.jsoftware.com/wiki/Vocabulary/curlyrtco https://code.jsoftware.com/wiki/Vocabulary/curlyrtco
  - floxy 6y ago
    Thanks. The "right way" to do max of the differences (with the sort) in J might be something like? >./-2-/\/:~ xs
teej 6y ago
My mind just cannot visualize what zip is doing. I’ve written production code with zip and I’ve finished leetcode problems with zip. But every single time I need a piece of paper to help me through it. I’m fine with most of the rest of Python’s functional-style iterable helpers. But something about zip just doesn’t work for me.
- Groxx 6y ago
  Zip is just "pair each x with each y". Literally: [1,2,3,4] [5,6,7,8] # go down each column, yielding these in order [1,5] [2,6] [3,7] [4,8] And then you drop ones with no pair. So zipping [1] and [2,3] makes just [1,2]. The same is true if you reverse the arguments, so [1,2] and [3] => [1,3] The example is sorta obtuse at a glance[1], but this: `zip(xs, xs[1:])` equates to this: [1,2,3,4] [2,3,4] # copy and drop the first element # outputs columns [1,2] [2,3] [3,4] So it's a one-liner to make a list of of each consecutive pair of items in a list. All `zip(x, x[1:])` do the same thing regardless of the ultimate use of such a construct. But yeah, I have seen some very confusing uses of zip before sitting and thinking for a bit. TBH I think much of it is just familiarity though - after seeing a million `for (i=0;i<thing.len();i++) { ... }` I can pattern-match and figure this out easily: for (i=0, l=thing.len(); i<l-1; i++) { list.push([thing[i], thing[i+1]]) } (though `i<l-1` ease may depend on font) but similarly-common functional idioms just don't slide into position as easily. Yet. [1]: I'm guessing it's the equivalent of finding the largest "gap" between a bunch of sizes of things.
- floxy 6y ago
  Think of a physical zipper, where the two sides with the teeth come together to interlock... xs = [1,2,3,4,5,6,... ys = [a,b,c,d,e,f,... [ (1,a), (2,b), (3,c), (4,d), (5,e), |6| |f| |7| |g| |8| |h| |9| |i| |0| |j| . . . . . . ] https://kezan.eu/wp-content/uploads/2016/05/Zipper_Thumb_1440.jpg https://kezan.eu/wp-content/uploads/2016/05/Zipper_Thumb_144...
f430 6y ago
I guess the second approach requires you to be more familiar with Python's concise syntax capabilities while the first is a nice waterfall model that anybody can read. Depends on your team. If you wrote stuff in the second way you would be obstructing, adding friction to less python experienced developers but you would be minimizing the amount of code written. It's hard to say, which is right or wrong here but the opacity of the second approach as you mention adds unpredictability. I honestly think Dart is a very good solution from the culmination of various classical languages we've been using so far with Java and Javascript but there just doesn't seem to be any major FANG company investing in creating something more "modern" with Python. I think it is a testament to Python's accessbility and also the depth it provides for experts as well. I guess the forced indentation is what gets to some developers but its a minor inconvenience.
TameAntelope 6y ago
Possibly dumb question, but why do type hints for a leetcode problem?
- ledauphin 6y ago
  i'd honestly ask, why not? when you paste this code, it's infinitely more readable out of its original context with those hints.
  - TameAntelope 6y ago
    I would argue it's less readable, substantially. Type hinting was never about readability, it was about getting IDEs/tools to do type checking for you. def max_width_of_vertical_area(points): xs = sorted(x for x, _ in points) return max(x2-x1 for x1, x2 in zip(xs, xs[1:])) Just seems easier...
    - haukilup 6y ago
      Although I see both sides, I personally lean towards finding that type hints improve readability. And since I tend towards consistency, that ends with most/all functions being typed. Though I spend most my time writing Swift code, which definitely influences my preference.
    - zo1 6y ago
      And then who-ever reads your code has no idea what "points" is. We had readable "type-annotation" 10+ years ago with comments to functions, this is just a logical extension to that. People need to know how to call your function, whether it's a statically-typed language or not. Would you say this is "less readable substantially" than your example? def max_width_of_vertical_area(points): # type points: List of List of int xs = sorted(x for x, _ in points) return max(x2-x1 for x1, x2 in zip(xs, xs[1:]))
      - pansa2 6y ago
        I would never comment that such a function takes a `List of List of Int` - I would instead describe `points` as being something like an `Iterable of Pairs of Numbers`. Are Python type hints able to specify types like that?
ianhorn 6y ago
A super common solution to the problem of too many ideas packed into a clever solution is just naming intermediate things. For example, naming the zip(...) is a little change that goes pretty far in making the second one more obvious: def maxWidthOfVerticalArea(self, points: List[List[int]]) -> int: xs = sorted(x for x, _ in points) neighboring_coordinates = zip(xs, xs[1:])) return max(right-left for left, right in neighboring_coordinates) or even naming the width generator: def maxWidthOfVerticalArea(self, points: List[List[int]]) -> int: xs = sorted(x for x, _ in points) neighboring_coordinates = zip(xs, xs[1:])) widths = (right-left for left, right in neighboring_coordinates) return max(widths) Same thing, just with more built in explanation. It's easy to go overboard, of course, but it's a useful go-to to make stuff more obvious.
- michaelhoffman 6y ago
  Whitespace helps too! And I think using `sorted(key=)` is more Pythonic than a generator expression ;) from operator import itemgetter def maxWidthOfVerticalArea(self, points: List[List[int]]) -> int: xs = sorted(points, key=itemgetter(0)) x_pairs = zip(xs, xs[1:]) widths = (right-left for left, right in x_pairs) return max(widths) NumPy also has stuff to solve this sort of thing built-in: from numpy import diff def maxWidthOfVerticalArea(self, points: List[List[int]]) -> int: xs = sorted(points, key=itemgetter(0)) widths = diff(xs) return widths.max()
  - ianhorn 6y ago
    For the last bunch of years, wherever my instinct says to add white space, I instead put a comment. If you want something to be a visually separate entity, it’s the perfect place to put a couple words about that entity, and it still gets the visual separation in an editor with any syntax highlighting at all.
  - roelschroeven 6y ago
    > And I think using `sorted(key=)` is more Pythonic than a generator expression ;) But it doesn't do the same thing. The original code extracted the xs from the coordinate pairs, sorted; your code sorts the coordinate pairs by their x values, but doesn't extract the xs from them.
at_a_remove 6y ago
Clever has its applications and its drawbacks. First off, you may not feel very clever when you need to read the code you wrote (no caffeine, or a hangover, or an incipient cold ...), or perhaps the next coder to pick up your code is not as clever. I save clever for the real mechanics of problems, for data structures, and occasionally optimization of running time if that is a factor (it often is not as much as you would think). I eschew clever if it seems like it is just a way to pack a lot of stuff into fewer lines of code. One of the reasons that Perl attained a reputation of "write once, read never" unmaintainable code (whether or not it was justified) is a culture that cultivates "one-liners" and packing everything down into a minimum space. Our filenames in Windows no longer have to be 8.3. We are not constrained to write our code in a few kilobytes. I programmed in the days where you might have two kilobytes in which to program. In those days, extreme terseness was a value. It is not any longer. "Cat" and "man" and "mv" are holdovers from those days. This is not a telegram where you are charged by the word. I program for total obviousness and sometimes that makes my code seem "dumb." I might do only one or two things per line. That's really fine. Just as an example, I wrote this obnoxious web app in Perl as part of my job. At some point, some students were to take it over, but they didn't know Perl. I received an excited phone call from them at some point: they were able to understand and port my application to a language they were familiar with because I commented, I was not clever in packing, and I kept it to "one or two things per line."
- crabmusket 6y ago
  To be fair, cat, man and mv are things you type many times per day when working in a unixy environment.
epr 6y ago
zip(xs[:-1], xs[1:]) # explicit zip(xs, xs[1:]) # implicit
Izkata 6y ago
I know a lot of people have jumped in already, but no one has come up with the intermediate version I think I'd've probably written (with one variable rename 'cause that's really long): def maxWidthOfVerticalArea(self, points: List[List[int]]) -> int: x_coords = sorted(x for x, _ in points) max_diff = 0 for x1, x2 in zip(x_coords, x_coords[1:]): max_diff = max(max_diff, x2 - x1) return max_diff
wodenokoto 6y ago
I found the second much easier to read. Particularly the for loop into the range of the length of the coords minus one, is difficult to wrap my head around and easy place for mistakes to sneak in. With that being said, I have had my difficulties wrapping my head around zip, and don't have difficulties anymore, which might actually be in favor of parents point.

panda88888 6y ago

I would guess the Pythonic approach is slower due to the slicing operation, which creates a new shallow copy of the original array.

squaresmile 6y ago

Interesting, I'm using this [0] to test and got different results. The Pythonic function is 2% slower on 3.9.1 AMD 4900HS but 17% faster on 3.9.1 E5-2660 v2 VPS and 6% faster on 3.6.9 i5-3570S bare metal. I personally think either function is fine. The max finding code pattern in the long function should be familiar to most. For the Pythonic function, I don't think it's less obvious enough to matter. You have a list of stuffs and you want to find the max so you wrap it in the max function. Maybe zip(xs, xs[1:]) can take a bit to recognize but I think it's eh. For the Pythonic one, one can also write all_diffs = [x2-x1 for x1, x2 in zip(xs, xs[1:])] return max(all_diffs) Using list is also markedly faster than the Pythonic generator for me, which is to be expected: ~16% faster than both. [0] https://gist.github.com/squaresmile/8dd08898851a4e19359cdb30e0f7b843 https://gist.github.com/squaresmile/8dd08898851a4e19359cdb30...

ALittleLight 6y ago
That is interesting. Trying your timeit code I've come to see that maybe something was wrong about how I was estimating the different performance. I was just using cProfile and profiling the two functions with big inputs and then comparing the cumulative time results that way. When I use your timeit examples my results (Intel Core i7-6700K CPU @ 4.00GHz) are different and closer to yours (pythonic ~1% slower). Thanks for sharing.

travisjungroth 6y ago

I just solved this problem yesterday and my code looked exactly like your second one. Even xs was the same, except I used a and b instead of x1 and x2 cause I always do that. But I actually like yours more. This is a very personal opinion, but I do like the second one more. I find it easier to read, but maybe that's just because I'm used to that style. And maybe you're more likely to have to rewrite it for a change, but that's kind of okay since there's less code there. On readability, here's how I'd read it out loud: Sort all the Xes from points. Return the max of the difference between two points that are next to each other. I find that quicker to put in my head than the equivalent parsing of the first solution, and exactly the intention of the code.

floxy 6y ago

FWIW, it was easier to me to follow your intent with the second example. The only clunky thing about is was the zip followed by the comprehension. It might be slightly nicer with something like: return max(xs[1:]-xs[:-1]) ...using something like numpy. I wonder if APL/J has successive element operator.

adamjb 6y ago
If you're gonna bring numpy into it you may as well use np.diff(): def maxWidthOfVerticalArea(self, points: List[List[int]]) -> int: return np.diff(sorted(x for x, _ in points)).max()
ahgamut 6y ago
J has the verbs behead[0] (for xs[1:]) and curtail[1] (for xs[:-1]), so the J oneliner for the above code would be <./ ([&}. - }:) xs [0] https://code.jsoftware.com/wiki/Vocabulary/curlyrtdot https://code.jsoftware.com/wiki/Vocabulary/curlyrtdot [1] https://code.jsoftware.com/wiki/Vocabulary/curlyrtco https://code.jsoftware.com/wiki/Vocabulary/curlyrtco
- floxy 6y ago
  Thanks. The "right way" to do max of the differences (with the sort) in J might be something like? >./-2-/\/:~ xs

teej 6y ago

My mind just cannot visualize what zip is doing. I’ve written production code with zip and I’ve finished leetcode problems with zip. But every single time I need a piece of paper to help me through it. I’m fine with most of the rest of Python’s functional-style iterable helpers. But something about zip just doesn’t work for me.

Groxx 6y ago
Zip is just "pair each x with each y". Literally: [1,2,3,4] [5,6,7,8] # go down each column, yielding these in order [1,5] [2,6] [3,7] [4,8] And then you drop ones with no pair. So zipping [1] and [2,3] makes just [1,2]. The same is true if you reverse the arguments, so [1,2] and [3] => [1,3] The example is sorta obtuse at a glance[1], but this: `zip(xs, xs[1:])` equates to this: [1,2,3,4] [2,3,4] # copy and drop the first element # outputs columns [1,2] [2,3] [3,4] So it's a one-liner to make a list of of each consecutive pair of items in a list. All `zip(x, x[1:])` do the same thing regardless of the ultimate use of such a construct. But yeah, I have seen some very confusing uses of zip before sitting and thinking for a bit. TBH I think much of it is just familiarity though - after seeing a million `for (i=0;i<thing.len();i++) { ... }` I can pattern-match and figure this out easily: for (i=0, l=thing.len(); i<l-1; i++) { list.push([thing[i], thing[i+1]]) } (though `i<l-1` ease may depend on font) but similarly-common functional idioms just don't slide into position as easily. Yet. [1]: I'm guessing it's the equivalent of finding the largest "gap" between a bunch of sizes of things.
floxy 6y ago
Think of a physical zipper, where the two sides with the teeth come together to interlock... xs = [1,2,3,4,5,6,... ys = [a,b,c,d,e,f,... [ (1,a), (2,b), (3,c), (4,d), (5,e), |6| |f| |7| |g| |8| |h| |9| |i| |0| |j| . . . . . . ] https://kezan.eu/wp-content/uploads/2016/05/Zipper_Thumb_1440.jpg https://kezan.eu/wp-content/uploads/2016/05/Zipper_Thumb_144...

f430 6y ago

I guess the second approach requires you to be more familiar with Python's concise syntax capabilities while the first is a nice waterfall model that anybody can read. Depends on your team. If you wrote stuff in the second way you would be obstructing, adding friction to less python experienced developers but you would be minimizing the amount of code written. It's hard to say, which is right or wrong here but the opacity of the second approach as you mention adds unpredictability. I honestly think Dart is a very good solution from the culmination of various classical languages we've been using so far with Java and Javascript but there just doesn't seem to be any major FANG company investing in creating something more "modern" with Python. I think it is a testament to Python's accessbility and also the depth it provides for experts as well. I guess the forced indentation is what gets to some developers but its a minor inconvenience.

TameAntelope 6y ago

Possibly dumb question, but why do type hints for a leetcode problem?

ledauphin 6y ago
i'd honestly ask, why not? when you paste this code, it's infinitely more readable out of its original context with those hints.
- TameAntelope 6y ago
  I would argue it's less readable, substantially. Type hinting was never about readability, it was about getting IDEs/tools to do type checking for you. def max_width_of_vertical_area(points): xs = sorted(x for x, _ in points) return max(x2-x1 for x1, x2 in zip(xs, xs[1:])) Just seems easier...
  - haukilup 6y ago
    Although I see both sides, I personally lean towards finding that type hints improve readability. And since I tend towards consistency, that ends with most/all functions being typed. Though I spend most my time writing Swift code, which definitely influences my preference.
  - zo1 6y ago
    And then who-ever reads your code has no idea what "points" is. We had readable "type-annotation" 10+ years ago with comments to functions, this is just a logical extension to that. People need to know how to call your function, whether it's a statically-typed language or not. Would you say this is "less readable substantially" than your example? def max_width_of_vertical_area(points): # type points: List of List of int xs = sorted(x for x, _ in points) return max(x2-x1 for x1, x2 in zip(xs, xs[1:]))
    - pansa2 6y ago
      I would never comment that such a function takes a `List of List of Int` - I would instead describe `points` as being something like an `Iterable of Pairs of Numbers`. Are Python type hints able to specify types like that?

ianhorn 6y ago

A super common solution to the problem of too many ideas packed into a clever solution is just naming intermediate things. For example, naming the zip(...) is a little change that goes pretty far in making the second one more obvious: def maxWidthOfVerticalArea(self, points: List[List[int]]) -> int: xs = sorted(x for x, _ in points) neighboring_coordinates = zip(xs, xs[1:])) return max(right-left for left, right in neighboring_coordinates) or even naming the width generator: def maxWidthOfVerticalArea(self, points: List[List[int]]) -> int: xs = sorted(x for x, _ in points) neighboring_coordinates = zip(xs, xs[1:])) widths = (right-left for left, right in neighboring_coordinates) return max(widths) Same thing, just with more built in explanation. It's easy to go overboard, of course, but it's a useful go-to to make stuff more obvious.

michaelhoffman 6y ago
Whitespace helps too! And I think using `sorted(key=)` is more Pythonic than a generator expression ;) from operator import itemgetter def maxWidthOfVerticalArea(self, points: List[List[int]]) -> int: xs = sorted(points, key=itemgetter(0)) x_pairs = zip(xs, xs[1:]) widths = (right-left for left, right in x_pairs) return max(widths) NumPy also has stuff to solve this sort of thing built-in: from numpy import diff def maxWidthOfVerticalArea(self, points: List[List[int]]) -> int: xs = sorted(points, key=itemgetter(0)) widths = diff(xs) return widths.max()
- ianhorn 6y ago
  For the last bunch of years, wherever my instinct says to add white space, I instead put a comment. If you want something to be a visually separate entity, it’s the perfect place to put a couple words about that entity, and it still gets the visual separation in an editor with any syntax highlighting at all.
- roelschroeven 6y ago
  > And I think using `sorted(key=)` is more Pythonic than a generator expression ;) But it doesn't do the same thing. The original code extracted the xs from the coordinate pairs, sorted; your code sorts the coordinate pairs by their x values, but doesn't extract the xs from them.

at_a_remove 6y ago

Clever has its applications and its drawbacks. First off, you may not feel very clever when you need to read the code you wrote (no caffeine, or a hangover, or an incipient cold ...), or perhaps the next coder to pick up your code is not as clever. I save clever for the real mechanics of problems, for data structures, and occasionally optimization of running time if that is a factor (it often is not as much as you would think). I eschew clever if it seems like it is just a way to pack a lot of stuff into fewer lines of code. One of the reasons that Perl attained a reputation of "write once, read never" unmaintainable code (whether or not it was justified) is a culture that cultivates "one-liners" and packing everything down into a minimum space. Our filenames in Windows no longer have to be 8.3. We are not constrained to write our code in a few kilobytes. I programmed in the days where you might have two kilobytes in which to program. In those days, extreme terseness was a value. It is not any longer. "Cat" and "man" and "mv" are holdovers from those days. This is not a telegram where you are charged by the word. I program for total obviousness and sometimes that makes my code seem "dumb." I might do only one or two things per line. That's really fine. Just as an example, I wrote this obnoxious web app in Perl as part of my job. At some point, some students were to take it over, but they didn't know Perl. I received an excited phone call from them at some point: they were able to understand and port my application to a language they were familiar with because I commented, I was not clever in packing, and I kept it to "one or two things per line."

crabmusket 6y ago
To be fair, cat, man and mv are things you type many times per day when working in a unixy environment.

epr 6y ago

zip(xs[:-1], xs[1:]) # explicit zip(xs, xs[1:]) # implicit

Izkata 6y ago

I know a lot of people have jumped in already, but no one has come up with the intermediate version I think I'd've probably written (with one variable rename 'cause that's really long): def maxWidthOfVerticalArea(self, points: List[List[int]]) -> int: x_coords = sorted(x for x, _ in points) max_diff = 0 for x1, x2 in zip(x_coords, x_coords[1:]): max_diff = max(max_diff, x2 - x1) return max_diff

wodenokoto 6y ago

I found the second much easier to read. Particularly the for loop into the range of the length of the coords minus one, is difficult to wrap my head around and easy place for mistakes to sneak in. With that being said, I have had my difficulties wrapping my head around zip, and don't have difficulties anymore, which might actually be in favor of parents point.