Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork7.9k
Fix lost ticks#1686
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Fix lost ticks#1686
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…roach that isused is to pad the interval that is being used to "clip" ticks from the axis becausethey are off the end.The padding is chosen specifically such that the tick cannot be drawn a pixel past theend of the axis. So long as the tick is within this bound, it should be OK to draw it.
… willgive locations one past what they really need to. This is necessary, becausein situations where round off error matters, we want the locator to offermore that it really thinks it needs to. The clipping of ticks still takesplace in the Axis class, so it's perfectly fine to add more locs than necessary.In fact, matplotlib relies on the clipping behavior in the Axis class alreadyto not draw ticks outside of the limits of the axis.
Interesting idea. I will have to take a closer look at its implications in |
I just saw that there are five fails in the Travis build. Two of these are by design...the MultipleLocator and LogLocator now return one more location on each end of the axis than they used to. The other three are image fails, and I don't know how to access the before/after images. |
Unfortunately, with Travis there isn't a way to access the images -- you just have to run the tests again locally when something fails. The other failure seems to be that the theta ticking on a polar plot is somehow broken. The one with the missing theta labels is from this branch -- the other is from the baseline images. |
Thanks Michael: I've installed nose and other dependencies, and I'm running the tests now. Good thing for the nose, or the polar thing would not have been caught ;) |
…fixed a problemin the calculation of the log tick locations.
On 2013/01/25 1:25 PM, dhyams wrote:
It's running whatever is installed.
|
Leaving a note here for github linkage foo: this PR is an alternative to PR#1659. |
the theta axis of a polar plot.
…n a distancein data coordinates, it should return 0.0 instead of 0 when appropriate.
Just a note -- the Travis failures seem to be identical to those on |
Right; they seem to be some weird difference in fonts; perhaps the reference images just need to be updated? |
On this one, is there anything else that I need to do? |
| @@ -1165,7 +1165,7 @@ def tick_values(self, vmin, vmax): | |||
| vmin = self._base.ge(vmin) | |||
| base = self._base.get_base() | |||
| n = (vmax - vmin + 0.001 * base) // base | |||
| locs = vmin + np.arange(n +1) * base | |||
| locs = vmin-base + np.arange(n +3) * base | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
As per PEP8, there should be spaces either side of the minus operator.
We're trying to standardize the codebase on PEP8, so would you mind going through and putting things like spaces between operators and after commas etc. Cheers, |
Sure, I'll modify it tonight and push again. I do wish, though, that there was a reasonable workflow such that others could make minor edits to changeset like this...in a fraction of the time required to post comments, it could have just been corrected to conform to pep8. Now I put it on my to-do list; pull up the vm when I get home, reread my git cheat sheet, make a couple of tiny edits, repush, verify that the changes made it, and await comment again. Seems like a very inefficient process, when it would be ideal just to make the edits right here in github. Just wondering if it were even possible. OK, enough of my whining ;) I don't mean it in a derogatory sense...just wondering if a more efficient process is available. |
You can edit directly on github, on your own branch. Just go to your fork (github.com/dhyams/matplotlib), choose the branch related to this pull request, browse to the file, press the Edit button... it will let you edit the commit message, and the changes will automatically show up in this pull request. As another time saver (which only works if you have the code checked out on your local machine), I usethis which adds a "open in editor" button toevery comment on github, which opens up the given file to the correct line in my editor. The example is with emacs, but there's nothing about the approach that couldn't be used with most other editors. |
PEP8 fixes, and part of a comment changed to a docstring.Also added a try/except around each call to _get_pixel_distance_along_axis, so thatif something goes wrong with some off-beat transform, we just don't pad; thingsrevert back to old matplotlib behavior.
Thank you mdboom; I had no idea that you could edit and commit like that. Of course, doing so makes it blindingly obvious that the changes are not tested, right ;) |
| @@ -1599,6 +1618,30 @@ def _get_offset_text(self): | |||
| self.offset_text_position = 'bottom' | |||
| return offsetText | |||
| def _get_pixel_distance_along_axis(self,where,perturb): | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Here and below, PEP8 spaces after commas, please.
@WeatherGod, you commented a month ago that you needed to check for 3D implications. Any thoughts on that, or any other aspect of this? It would be good to either get this moving, or reject it. |
…ctly howmuch padding to add.
the _get_pixel_distance routines. Also added a warning emit, so thatthe try/catch at least says something if it is triggered.
Doesn't seem to have any negative impacts for mplot3d, as far as I can see. Haven't really had the time to examine the change in-depth, but I think the basic idea is sound. |
Let's see how it works in practice. |
Inmatplotlib#1686 this was set to half a pixel, but I don't think this makessense, and it is not needed in order to pass the test that wasadded to check for lost ticks (matplotlib#1310), or for the cases inmatplotlib#1310.
Inmatplotlib#1686 this was set to half a pixel, but I don't think this makessense, and it is not needed in order to pass the test that wasadded to check for lost ticks (matplotlib#1310), or for the cases inmatplotlib#1310.
For issue#1310: Drops last tick label for some ranges
Added a strategy to stop losing ticks on the ends of an axis. The approach that is
used is to pad the interval that is being used to "clip" ticks from the axis because
they are off the end.
The padding is chosen specifically such that the tick cannot be drawn a pixel past the
end of the axis. So long as the tick is within this bound, it should be OK to draw it.