Add RuntimeWarning guard around division-by-zero #22628
Conversation
1296ee9 to
59b520d
Compare
|
This comes up in the degenerate case when |
|
I would go with a test of the code from #22624. The thing we care about is that that code should not warn. |
When I debugged it I noticed that x21 and y21 became zero quite often, but you are right, looking back in the code that is what happens. I'll modify it. |
As I get it, one will need to move the mouse and it is when the mouse enters the window that the first warning is emitted. We can of course test the |
19d7c4a to
4cb1dbd
Compare
|
@anntzer had a relevant comment in #22624 (comment) Maybe one should simply not plot the mouse pointer position when in 3D mode? (Still, if that is not decided and implemented, this can cover in the meanwhile.) |
4cb1dbd to
1160584
Compare
| @@ -17,11 +17,13 @@ def _line2d_seg_dist(p1, p2, p0): | |||
| and intersection point lies within segment if u is between 0 and 1 | |||
| """ | |||
There was a problem hiding this comment.
Your solution is technically correct. However, the issue is more a logical one rather than a technical division-by-zero problem. Therefore, I'd go with the following and leave all the original code unchanged in place.
| """ | |
| If p1 and p2 are identical, the distance between them and p0 is returned. | |
| """ | |
| if np.all(p1 == p2): | |
| return np.hypot(p1[0] - p0[0], p1[1] - p0[1]) |
There was a problem hiding this comment.
Added the docs and removed the comment. (Kept the solution with intermediate results.)
There was a problem hiding this comment.
Kept the solution with intermediate results.
@oscargus: What is the motivation for keeping this? It would be nice to give a reason if you dismiss a suggestion. I find the separation of the special case and using explicit values there more clear, but can be convinced otherwise.
There was a problem hiding this comment.
Just wanted to avoid duplicate code (and was a bit unsure what the effect of np.asarray was).
But I see your point about separation as well. And will try to motivate better in the future.
1160584 to
503d7b9
Compare
| @@ -1002,6 +1002,16 @@ def test_lines_dists(): | |||
| ax.set_ylim(0, 300) | |||
|
|
|||
|
|
|||
| def test_lines_dists_nowarning(): | |||
| # Smoke test to see that no RuntimeWarning is emitted when two first | |||
There was a problem hiding this comment.
would it be preferable to test a user-facing API rather than the private methods?
There was a problem hiding this comment.
This is mostly called through mouse callbacks so it is annoying to get public API tests that exercise this.
Where to bail out for equal points is somewhat a stylistic choice. Not blocking over the current state.
…628-on-v3.5.x Backport PR #22628 on branch v3.5.x (Add RuntimeWarning guard around division-by-zero)
PR Summary
Closes #22624
Avoid a RuntimeWarning otherwise plotted at the first divide by zero.
Not sure if this can be tested properly. Directly calling the function is one way, but I do not have any other ideas really.
PR Checklist
Tests and Styling
pytestpasses).flake8-docstringsand runflake8 --docstring-convention=all).Documentation
doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).