| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
Try panning around the Axes in that example; it does not work the same with and without smart bounds. |
Sorry, something went wrong.
|
If set_smart_bounds stays it’d be nice if the docstring was a little more informative as to what a smart bound is. |
Sorry, something went wrong.
|
I've only looked at one failing test, but I suspect it will be characteristic. I think the problem is with the old behavior, not the new behavior, but this can be discussed. In specgram_noise, the actual xlims are |
Sorry, something went wrong.
|
@QuLogic, does the smart_bounds behavior add any real value? If so, what is the desired behavior that it uniquely provides? |
Sorry, something went wrong.
|
I don't have any particular attachment to it; just pointing out that it is not at all the same as Axes.autoscale_view. |
Sorry, something went wrong.
|
@QuLogic You are right, of course; I was not looking carefully at what the smart_bounds was doing. The big effect is that it keeps the spines locked to the data as the data are moved, so that the spine spans the part of the data range that fits within the xlim and ylim instead of spanning those limits as they are changed. If anyone wants to test this, the modified excerpt from spine_placement_demo.py will serve: import numpy as np
import matplotlib.pyplot as plt
plt.rcParams['axes.autolimit_mode'] = 'round_numbers'
plt.rcParams['axes.xmargin'] = 0
plt.rcParams['axes.ymargin'] = 0
for smart in (True, False):
fig, ax = plt.subplots()
fig.suptitle('smart is %s' % smart)
x = np.linspace(-np.pi, np.pi, 100)
y = 2 * np.sin(x)
ax.set_title('centered spines')
ax.plot(x, y)
ax.spines['left'].set_position('center')
ax.spines['right'].set_color('none')
ax.spines['bottom'].set_position('center')
ax.spines['top'].set_color('none')
ax.spines['left'].set_smart_bounds(smart)
ax.spines['bottom'].set_smart_bounds(smart)
ax.xaxis.set_ticks_position('bottom')
ax.yaxis.set_ticks_position('left')
plt.show() |
Sorry, something went wrong.
|
@astraw, would you explain the use case for smart_bounds, please? |
Sorry, something went wrong.
|
Seems cute, but I don't see what the data analysis/presentation use-case might be. |
Sorry, something went wrong.
|
I wrote the original "dropped spine" implementation (which John Hunter noted sounded like a medical condition) and this looks like the translated-to-git commit: 43ca13b. The word "smart" is not in that commit nor do I remember any smart bounds feature. So without doing further spelunking, I cannot offer any support of smart_bounds. If you are in favor of removing in your cleanup, please go ahead. |
Sorry, something went wrong.
|
There is an example with dropped spines and it does not use this smart bounds feature, so I'm not sure how they were related. |
Sorry, something went wrong.
|
In my original dropped spines implementation I wrote an example which showed the various different interaction modes which I tested with. This was the primary test case that I developed the code for. If that still runs without problems and with each axes behaving differently (there was no redundancy), then all my original use cases are accounted for. (I'm super busy this moment and haven't looked to check what has become of that example.) |
Sorry, something went wrong.
|
@astraw, I appreciate your responding and don't want to burden you. The history as transferred from svn to github includes the following. |
Sorry, something went wrong.
|
Thanks for the summary. What you write is consistent with my memory in that I did spend some effort in tuning interactive behavior and also tuning the length of the spines to end at data limits. So if "smart bounds" is indeed the name of the feature to cause the spines to end at the data limits, I would say this is a useful feature, and not just for interactive mode. In any case, I haven't followed MPL development closely enough to have an informed opinion about this and would defer to the devs' preference on this. I certainly can understand if a simple code base is seen to be more important than obscure features. |
Sorry, something went wrong.
|
Rebased and updated. |
Sorry, something went wrong.
|
This needs a few test images updated... |
Sorry, something went wrong.
|
Thanks. I had it all staged, just needed to commit and push. Unfortunately, I think these test images are much larger than they need to be for their real purpose. They all lose an initial tick with this PR because their first x data point is a small positive number, not zero. |
Sorry, something went wrong.
Is that clear? <0.5px makes sense in that it means that if e.g. you draw without antialiasing, then the tick will be aligned with the edge (well... perhaps, depending on where you fall exactly with the pixel edges). In general shouldn't this really (as usual) be handled by the locators deciding exactly what ticks to show? (Per the argument made again and again...) The locators already have their own tolerance mechanism for floating point error, cf the Base class that you fiddled with as well IIRC. Also I did some quick profiling a while ago and IIRC this _get_pixel_distance_along_axis is actually contributing >~1% to the total runtime, due to the need to construct, well, a bunch of transforms along the way. So it'd be nice to see it go away. Note that I'm not blocking this PR on that change, I guess it's still an improvement, just making sure I understand what's going on. |
Sorry, something went wrong.
|
@efiring, why don't we trim the ticks in tick_values? If I add the code below the results seem good to me. A bunch of ticker tests fail, but thats because they were returning ticks that are out of bounds. def tick_values(self, vmin, vmax):
if self._symmetric:
vmax = max(abs(vmin), abs(vmax))
vmin = -vmax
vmin, vmax = mtransforms.nonsingular(
vmin, vmax, expander=1e-13, tiny=1e-14)
locs = self._raw_ticks(vmin, vmax)
# make sure ticks are inside vmin, vmax within numerical tolerance:
rtol = np.abs(vmax - vmin) * 1e-12
locs = locs[locs < vmax + rtol]
locs = locs[locs > vmin - rtol]
prune = self._prune
if prune == 'lower':
locs = locs[1:]
elif prune == 'upper':
locs = locs[:-1]
elif prune == 'both':
locs = locs[1:-1]
return self.raise_if_exceeds(locs) |
Sorry, something went wrong.
|
I think this was superceded by #12158. Closing, but please do re-open if I'm mistaken... |
Sorry, something went wrong.
|
No, I think this is mostly unrelated to #12158. |
Sorry, something went wrong.
|
Sorry... OK to punt it to 3.2 or are you hoping to come back to it? |
Sorry, something went wrong.
|
Punted. |
Sorry, something went wrong.
|
This still needs an api_changes entry. This PR is highly modified from the original, so that now it's sole purpose is to deprecate the smart_bounds feature. I could turn it into a new PR and simply refer from there to the extensive comments here about that feature. |
Sorry, something went wrong.
There was a problem hiding this comment.
Looks good to me. I'll change the PR title if thats OK...
Sorry, something went wrong.
|
How are deprecations added to the "what's new" documentation? Is this done at release time by the release manager? It doesn't appear to be done via adding an rst file to the doc/api/api_changes directory. |
Sorry, something went wrong.
|
You put an entry in doc/api/next_api_changes |
Sorry, something went wrong.
|
I just wanted to note that the plotting module in SymPy relies on these smart_bounds calls and we will need an alternative if they are removed from matplotlib. |
Sorry, something went wrong.
|
Could you give us some context about how your are using smart bounds? |
Sorry, something went wrong.
|
From the SymPy 18870 referenced above, it appears that there is exactly one line that was setting smart_bounds for a spine to True, hence activating that logic path. Based on the change in plots in https://docs.sympy.org/dev/modules/plotting.html, it appears that this was somehow controlling the ylim. I don't know why this can't be done either with normal mpl autoscaling or by explicitly setting ylim as desired within SymPy. |
Sorry, something went wrong.
|
I've dug a little deeper and I don't think the smart_bounds was the direct cause of the output changes in SymPy's plotting functions. I've opened #17004 that points to a regression in mpl 3.2 from 3.1 that is causing the effect. |
Sorry, something went wrong.
|
Given that, we will need a workaround for the removal of smart_bounds in SymPy. Trying to figure out what that was is clouded by the possible unrelated regression. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
PR Summary
Code related to spines, ticks, and autoscaling needs some simple cleanups, which might be followed by some deeper changes. To begin:
Edited, 2019-07-16: Everything but the last item has been superceded by other PRs, and therefore has been stripped out of this one.
PR Checklist