Skip to content

Make sure that initial state gets reset if anything goes wrong in rc_context [backport to 1.4.x] #3752

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 3 commits into from
Nov 3, 2014

Conversation

astrofrog
Copy link
Contributor

Fixed a bug that caused rc parameters to not be reset to initial state if an exception happened while setting new rc parameters inside rc_context. This matters because I use rc_context is a testing suite and one of my tests may want rc_context to fail, but then the rcparams are left in a modified state for future tests. I have included a regression test.

Unfortunately I have to use an ordered dict otherwise the failure is not deterministic (because it depends on the order in which the rcparams are set) so if ordered dict can't import I simply pass the test (I wasn't sure if you make use of the skipping plugin).

…e if an exception happened while setting new rc parameters inside rc_context.
rcParams.update(self.rcdict)
except:
# if anything goes wrong, revert rc parameters and re-raise
rcParams.update(self._rcparams)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Technically speaking, wouldn't this still leave new entries in the dictionary? Of course, this might not matter if the set of entries is always the same. Still I would have hoped that there would be a context manager for something like this already. If there isn't, it feels like a hole in the standard library.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@WeatherGod - ah yes, good catch, I've updated this to add a call to clear before the update, which I think will do the right thing.

@astrofrog
Copy link
Contributor Author

@WeatherGod - I've implemented your suggestions

try:
from collections import OrderedDict
except ImportError:
return # can't run this test on Python 2.6
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a tad picky, can you raise a skip here and do it by testing the version of python? There are a couple of other tests in this file that do the same thing.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm fine with picky :) I pushed a new change.

@tacaswell tacaswell modified the milestones: v1.5.x, v1.4.3 Nov 3, 2014
@tacaswell tacaswell changed the title Make sure that initial state gets reset if anything goes wrong in rc_context Make sure that initial state gets reset if anything goes wrong in rc_context [backport to 1.4.x] Nov 3, 2014
tacaswell added a commit that referenced this pull request Nov 3, 2014
BUG : Make sure that initial state gets reset if anything goes wrong in ``rc_context``
@tacaswell tacaswell merged commit 112f199 into matplotlib:master Nov 3, 2014
tacaswell added a commit that referenced this pull request Nov 3, 2014
BUG : Make sure that initial state gets reset if anything goes wrong in ``rc_context``
@tacaswell
Copy link
Member

Cherry-picked as 001b1e9

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