Sync 3D errorbar with 2D#18436
Merged
tacaswell merged 5 commits intomatplotlib:masterfrom Feb 19, 2021
Merged
Conversation
6 tasks
tacaswell
reviewed
Oct 14, 2020
Member
|
I am 👍 on this PR. |
53028b4 to
2f2cc5a
Compare
Member
Author
|
I'll call this done on 80%/20%, but might have a followup later. |
7 tasks
tacaswell
reviewed
Feb 17, 2021
tacaswell
reviewed
Feb 17, 2021
tacaswell
approved these changes
Feb 17, 2021
Member
tacaswell
left a comment
There was a problem hiding this comment.
👍
left one super minor comment.
Member
|
If no one else reviews this in the next 6 or so hours I am going to merge this on a single review. |
jklymak
reviewed
Feb 18, 2021
Member
jklymak
left a comment
There was a problem hiding this comment.
I guess I'm concerned that a lot of this is untested... But not concerned enough to block.
anntzer
approved these changes
Feb 18, 2021
Contributor
anntzer
left a comment
There was a problem hiding this comment.
modulo test change discussed today.
These variables are not used within these `if` blocks, and aren't meant to be used outside of them either.
Namely, kwarg normalization, unit info processing, and adding a default `data_line`, so it works when `fmt='none'`. Move iterable checks before working on styling. Also, move `errorevery` checks to a similar location as the 2D code.
As with 2D before matplotlib#17930, it would not cycle if a color were specified. However, this does not match `plot`, which does not advance the cycle only if _all_ properties in the cycle are specified. Notably, this means if your property cycle was for line style, specifying a color would ignore the cycle in `errorbar`, but not in `plot`. This is a 3D version of 149e7fb and 0782c74.
There are a few differences that cause some image changes: * When both upper and lower limits are True, `errorbar3d` incorrectly used full errorbar length for them. They should both have 0 errorbar with the arrow-head cap. * The arrow-head cap should use `eb_cap_style`, not `eb_lines_style`. This meant that the capsize defaulted to 0, so this was made explicitly non-zero in the test. * The baseline of the triangle (bottom/top for caps above/below a line) in 2D is aligned with the end of the errorbar, *not* the tip of the triangle, so all quivers in 3D shifted outward to match. * The quiver would preferably not overlap the existing errorbar, which I've hopefully achieved by setting the length based on `capsize`, and using the above positioning. Consequently, `arrow_length_ratio` is no longer exposed.
Member
Author
|
For reference on the change in
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR Summary
This fixes some inconsistencies of the 3D
errorbarWRT the 2D version. Some of this is code rearrangement and cycler handling to match #17930.Additionally, there's some inconsistency with caps for limited sides. Previously, they were drawn from a point on the line to the errorbar length with the arrow head ending on it. In 2D, the triangle marker starts at the errorbar length. As a consequence of fixing this, the
arrow_length_ratiois no longer exposed. Also, when both limits are disabled, the 2D code places both caps at zero offset, but 3D placed them at default errorbar length (this is the case for the second-last errorbar in the test image).All that being said, the size of arrows for caps are currently calculated somewhat arbitrarily in order to match the look of the test image. Unlike 2D, the 3D quiver arrows are not sized for display, but in data space. So I need to investigate that, and this will currently be marked as draft.
PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsandpydocstyle<4and runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).