Skip to content

Fix divide by 0 runtime warning #23430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 18, 2022

Conversation

scottshambaugh
Copy link
Contributor

@scottshambaugh scottshambaugh commented Jul 15, 2022

PR Summary

PR #22624 didn't fix the runtime error popping up when the two points have the same X and Y but differing Z coordinates. This fixes that.

Traceback (most recent call last):
  File "/mnt/c/Users/Scott/Documents/Documents/Coding/matplotlib/lib/mpl_toolkits/mplot3d/proj3d.py", line 30, in _line2d_seg_dist
    u = (x01*x21 + y01*y21) / (x21**2 + y21**2)
FloatingPointError: invalid value encountered in double_scalars

PR Checklist

Tests and Styling

  • Has pytest style unit tests (and pytest passes).
  • Is Flake 8 compliant (install flake8-docstrings and run flake8 --docstring-convention=all).

Documentation

  • [N/A] New features are documented, with examples if plot related.
  • [N/A] New features have an entry in doc/users/next_whats_new/ (follow instructions in README.rst there).
  • [N/A] API changes documented in doc/api/next_api_changes/ (follow instructions in README.rst there).
  • [N/A] Documentation is sphinx and numpydoc compliant (the docs should build without error).

@scottshambaugh scottshambaugh changed the title Fix div0 runtime error Fix div0 runtime warning Jul 15, 2022
@scottshambaugh scottshambaugh changed the title Fix div0 runtime warning Fix divide by 0 runtime warning Jul 15, 2022
@tacaswell tacaswell added this to the v3.5.3 milestone Jul 16, 2022
p1 = (10, 30, 20)
p2 = (20, 150)
proj3d._line2d_seg_dist(p0, p0, p2)
proj3d._line2d_seg_dist(p0, p1, p2)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Um, this now accepts a 3d line and a 2d point? I'm not clear what the semantics here is or whether this is desired. At the very least we should document the expected input dimensions for the function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an internal helper that I think we pulled out.

Given that @scottshambaugh was seeing this warning we are apparently passing in 3D points in practice.

While this is an odd internal wart, I do not think it is worth the effort we have collectively thrown at this function recently!

Copy link
Member

@timhoffm timhoffm Jul 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My point is that we are doing some ill-defined things in the function. Note that this function is only used in format_coord and I wouldn't be surprised if it's the cause of #22775.

The earlier work #22628 was defensive in only bailing out when both line-defining points were equal in 3d. That does not alter the behavior - it's not making anything worse at least.

I don't know what the correct behavior here should be, but I believe that neglecting the z-direction will add another quirk to the coord determination.
Edit: Rethinking this, since we are anyway only measuring the distance in x, y. This check is not making it worse either. - As little as that is worth.

More generally, I support @anntzer's view that we should stop displaying 3D coords, because we can't get them from a 2d pointer consistently.

@timhoffm timhoffm dismissed their stale review July 18, 2022 00:35

This is not making things worse than they are. So ok.

@timhoffm timhoffm merged commit 2716c82 into matplotlib:main Jul 18, 2022
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Jul 18, 2022
timhoffm added a commit that referenced this pull request Jul 18, 2022
…430-on-v3.5.x

Backport PR #23430 on branch v3.5.x (Fix divide by 0 runtime warning)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants