Skip to content

FIX: do not always reset scales from figureoptions #6281

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
May 16, 2016

Conversation

tacaswell
Copy link
Member

Only try to set the axis scales if they have been changed.

closes #6276

The set_yscale and set_xscale Axes methods call out to
Axis_set_scale which resets the default locator/formatters (which
makes sense when flipping between log/linear).

Putting the cut-out in figureoption is the less disruptive change,
even if the case could be made that the no-op short-circuit should be
done in Axis or Axes.

Only try to set the axis scales if they have been changed.

closes matplotlib#6276

The `set_yscale` and `set_xscale` Axes methods call out to
`Axis_set_scale` which resets the default locator/formatters (which
makes sense when flipping between log/linear).

Putting the cut-out in `figureoption` is the less disruptive change,
even if the case could be made that the no-op short-circuit should be
done in Axis or Axes.
@tacaswell tacaswell added this to the 1.5.2 (Critical bug fix release) milestone Apr 8, 2016
@QuLogic QuLogic changed the title FIX: do not always reset scales from igureoptions FIX: do not always reset scales from figureoptions Apr 8, 2016
@tacaswell
Copy link
Member Author

@anntzer Can you review this?

@anntzer
Copy link
Contributor

anntzer commented Apr 29, 2016

The remaining issue is that the formatting is lost after changing the scale from linear to log and back to linear again. Intuitively I feel like scale objects should be cached (most likely in a name->scale dict attached to the axis object) so that formatting is not lost... but perhaps this would be overengineering it? (Also, one would need a way to bypass the cache, most likely by passing an explicit scale object to set_scale.)

@tacaswell
Copy link
Member Author

That definitely feels like overengineering to me. If someone did want to keep track of the full history of the formatters/locators then it should live in a layer above base mpl.

@anntzer
Copy link
Contributor

anntzer commented Apr 29, 2016

My gut feeling/expectation is that after creating a figure programatically, any changes done to it from the figure options UI should be reversible, which is why I proposed the scale object cache.

Where to put the no-op check comes down to what behavior you expect for something like

ax.set_xscale("log")
<mess with formatter options>
ax.set_xscale("log")

Ignoring the implementation right now, what do you think about this?

@dopplershift
Copy link
Contributor

@tacaswell this appears to be waiting on feedback from you.

@tacaswell
Copy link
Member Author

I would expect that given a set of artists on in the axes ax.set_xscale() should always put the figure back into the same state, independent of past history. Once we go down the route of remember this sort of thing we will end up turning every property into a time series with matching heuristics.

One thing that will help here is once we have traitlets, setting the default values of the properties on the LogFormatter and LogLocator will be easy.

@anntzer
Copy link
Contributor

anntzer commented May 13, 2016

That's fair. In this case, I'd suggest a comment in the definition of set_xscale and friends warning that setting the scale to its current value may undo other changes (possibly pointing to this diff as an example).

@mdboom mdboom merged commit 7a80933 into matplotlib:v1.5.x May 16, 2016
@tacaswell tacaswell deleted the fix_qt_setscales branch May 16, 2016 21:53
anntzer pushed a commit to anntzer/matplotlib that referenced this pull request Jul 24, 2016
Only try to set the axis scales if they have been changed.

closes matplotlib#6276

The `set_yscale` and `set_xscale` Axes methods call out to
`Axis_set_scale` which resets the default locator/formatters (which
makes sense when flipping between log/linear).

Putting the cut-out in `figureoption` is the less disruptive change,
even if the case could be made that the no-op short-circuit should be
done in Axis or Axes.

Closes matplotlib#6281
@anntzer anntzer mentioned this pull request Nov 30, 2016
2 tasks
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.

4 participants