Skip to content
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

ticker.LinearLocator view_limits algorithm improvement closes #6142 #6146

Merged
merged 1 commit into from May 4, 2016

Conversation

maqifrnswa
Copy link
Contributor

No description provided.

@tacaswell tacaswell added this to the 2.0 (style change major release) milestone Mar 12, 2016
@tacaswell
Copy link
Member

attn @mdboom

The conditions where this applies are a) if rcParams['axes.autolimit_mode'] == 'round_numbers which was added as a back-compatibility mode in 2.0 and b) the number of ticks in the locator != 11.

This is technically an API break, but could be convinced this is enough of a corner case / no one really wants the old behavior to let it slide.

@maqifrnswa Can you add some before/after example of what this looks like?

@tacaswell tacaswell modified the milestones: 2.1 (next point release), 2.0 (style change major release) Mar 14, 2016
@tacaswell
Copy link
Member

How did you end up using LinearLocator? We have (for the most part) deprecated the use of this locator internally in favor of MaxNLocator which tries to pick both nice limits and a nice number of ticks which helps to alleviate some of these issues.

I played with this a bit and could not see that this was clearly better than the current behavior.

Pushing this off to 2.1 as it is not a blocking issue for 2.0.

Can you provide some before/after examples and add a note to the api_changes folder?

@maqifrnswa
Copy link
Contributor Author

The use case is for lining up the grid lines of a plot with two y-axis. When you are using twinx() with gridlines, you need the exact same number of ticks on both y-axes so the grids line up. MaxNLocator does not guarantee the exact number of ticks on each axis, so you have to use LinearLocator. It might be good to add a note in the twinx documentation to let people know about LinearLocator. I can put together notes to api_changes and some text in twinx making users aware of this change so they don't have to resort to hacks (which is what is done in the references below):

References with example figures:
http://stackoverflow.com/questions/26752464/matplotlib-two-y-axis-scales-how-to-align-gridlines/35904761#35904761

http://stackoverflow.com/questions/20243683/matplotlib-align-twinx-tick-marks

@tacaswell
Copy link
Member

Ah, that use case is compelling.

Can you give any examples of data ranges where this makes a big difference?

This needs a rebase at any rate.

@maqifrnswa
Copy link
Contributor Author

Do you think twinx plots should default to this proposed linearlocator?

To show examples of data ranges where this makes a difference, here's a script to generate lists of tick locations versus numticks and data size:
https://gist.github.com/maqifrnswa/5d23546f9314f15cd39c228101eadf1c

Here is for sample ranges of 20-90 and numticks from 3 to 11
The biggest changes in this example are for numticks >11 and for numticks = 10, 9, 7, and 4.

numticks: 12, data min: 20, data max: 90
Current:
[ 20. 26.36363636 32.72727273 39.09090909 45.45454545
51.81818182 58.18181818 64.54545455 70.90909091 77.27272727
83.63636364 90. ]
Proposed:
[ 11. 19. 27. 35. 43. 51. 59. 67. 75. 83. 91. 99.]

numticks: 10, data min: 20, data max: 90
Current:
[ 20. 27.77777778 35.55555556 43.33333333 51.11111111
58.88888889 66.66666667 74.44444444 82.22222222 90. ]
Proposed:
[ 18. 26. 34. 42. 50. 58. 66. 74. 82. 90.]

numticks: 9, data min: 20, data max: 90
Current:
[ 20. 28.75 37.5 46.25 55. 63.75 72.5 81.25 90. ]
Proposed:
[ 16. 26. 36. 46. 56. 66. 76. 86. 96.]

numticks: 8, data min: 20, data max: 90
Current:
[ 20. 30. 40. 50. 60. 70. 80. 90.]
Proposed:
[ 14. 25. 36. 47. 58. 69. 80. 91.]

numticks: 7, data min: 20, data max: 90
Current:
[ 20. 31.66666667 43.33333333 55. 66.66666667
78.33333333 90. ]
Proposed:
[ 18. 30. 42. 54. 66. 78. 90.]

numticks: 6, data min: 20, data max: 90
Current:
[ 20. 34. 48. 62. 76. 90.]
Proposed:
[ 20. 34. 48. 62. 76. 90.]

numticks: 5, data min: 20, data max: 90
Current:
[ 20. 37.5 55. 72.5 90. ]
Proposed:
[ 16. 36. 56. 76. 96.]

numticks: 4, data min: 20, data max: 90
Current:
[ 20. 43.33333333 66.66666667 90. ]
Proposed:
[ 18. 42. 66. 90.]

numticks: 3, data min: 20, data max: 90
Current:
[ 20. 55. 90.]
Proposed:
[ 0. 48. 96.]

Finally, no change if you use the current default number of ticks:
numticks: 11, data min: 20, data max: 90
Current:
[ 20. 27. 34. 41. 48. 55. 62. 69. 76. 83. 90.]
Proposed:
[ 20. 27. 34. 41. 48. 55. 62. 69. 76. 83. 90.]

@efiring
Copy link
Member

efiring commented Mar 29, 2016

On 2016/03/29 10:34 AM, Scott Howard wrote:

Do you think twinx plots should default to this proposed linearlocator?

I think the way to expose it should be not as a default, but via kwargs
to select the locator and the target or required number of bins.

@tacaswell
Copy link
Member

I like it (and even understand the implementation now!).

Given that this is an API break but not strictly a new feature, I think this should be pulled into v2.x.

The choice of 1 vs min(self.num_ticks-1, 1) should be controlled by _internal.classic_mode kwarg.

@tacaswell tacaswell modified the milestones: 2.0 (style change major release), 2.1 (next point release) Apr 30, 2016
@tacaswell tacaswell merged commit 6bed1be into matplotlib:master May 4, 2016
@tacaswell
Copy link
Member

@maqifrnswa Can you do the twinx/twiny changes in a follow up PR?

@QuLogic
Copy link
Member

QuLogic commented May 4, 2016

Given that this is an API break but not strictly a new feature, I think this should be pulled into v2.x.

@tacaswell tacaswell modified the milestone: 2.0 (style change major release), 2.1 (next point release) 4 days ago

I'm a bit confused; one side says 2.0, the other says 2.1.

@QuLogic
Copy link
Member

QuLogic commented May 4, 2016

Oops, I meant to refer to #6142 there. No backports noted just yet.

@tacaswell
Copy link
Member

I think this should be back ported to 2.x.

On Wed, May 4, 2016, 03:27 Elliott Sales de Andrade <
notifications@github.com> wrote:

Oops, I meant to refer to #6142
#6142 there. No
backports noted just yet.


You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#6146 (comment)

@QuLogic
Copy link
Member

QuLogic commented May 12, 2016

I don't think this was backported...

tacaswell added a commit that referenced this pull request May 13, 2016
API: ticker.LinearLocator view_limits algorithm changes

closes #6142
@tacaswell
Copy link
Member

(finally) backported to v2.x as e785afb

@tacaswell
Copy link
Member

@QuLogic Thanks for keeping track of this!

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.

None yet

5 participants