mep12 on quiver_demo.py #4833
mep12 on quiver_demo.py #4833
Conversation
| 1, | ||
| r'$1 \frac{m}{s}$', | ||
| fontproperties={ | ||
| 'weight': 'bold'}) |
There was a problem hiding this comment.
still need to fix the autopep8 issue above.
| dx, dy = r - l, t - b | ||
| axis([l - 0.05*dx, r + 0.05*dx, b - 0.05*dy, t + 0.05*dy]) | ||
| plt.axis([l - 0.05 * dx, r + 0.05 * dx, b - 0.05 * dy, t + 0.05 * dy]) |
There was a problem hiding this comment.
It looks like this step is creating a 5% margin. Unfortunately, it does not look like you would get the same effect if you do plt.margins(0.05) instead of the previous 3 lines involving axis()... I wonder if that is a bug?
There was a problem hiding this comment.
BTW, I would not add spaces around the asterisk.
There was a problem hiding this comment.
Ok, not a bug as what this code is doing is not exactly the same as what margins() does. I can get it to be fairly close using plt.margins(0.05, tight=False), but it isn't the same. So I guess we will just leave it as is.
As for the spaces around the asterisks, it probably was clearer before. This is the danger of autopep8! The only time I use it is when I am reworking someone else's code who has very little experience with proper code formatting. The intent is not that the autopep8 version gets into version control, the intent is to make it easier for me to read so that I can rewrite it better. Even then, I don't let it run with all of the options on.
| # 6 | ||
| plt.figure() | ||
| M = np.zeros(U.shape, dtype='bool') | ||
| M[U.shape[0] / 3:2 * U.shape[0] / 3, U.shape[1] / 3:2 * U.shape[1] / 3] = True |
There was a problem hiding this comment.
IMHO the line looks better without the spaces around the operators.
There was a problem hiding this comment.
Agreed. In this version, I see the 3:2 grouped together, even though they are separate. If anything, I would instead have each slice on their own lines. This would also keep it under 80 chars.
|
PEP8 failures: And I agree with that error. What I recommend doing is to split the indexing across two lines at the comma. That will make it more readable. In addition, you can use parentheses to help visually group things if it helps, but sometimes parentheses just add visual clutter. You also still have some cleanup from the autopep8 gone awry to deal with. |
|
Got it, I will try parentheses and splitting the lines. And avoid autopep8 for this file. |
|
Looks like it's pep8 issues. Gonna fix. |
No description provided.