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

Disable caching of renderer parameters on OutputMagic #1285

Merged
merged 12 commits into from
Apr 13, 2017
22 changes: 14 additions & 8 deletions holoviews/ipython/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -146,15 +146,22 @@ class notebook_extension(param.ParameterizedFunction):
'plotly': 'plotly'}

def __call__(self, *args, **params):
imports = [(name, b) for name, b in self._backends.items()
if name in args or params.get(name, False)]
if not imports or 'matplotlib' not in Store.renderers:
imports = imports + [('matplotlib', 'mpl')]
# Get requested backends
imports = [(arg, self._backends[arg]) for arg in args
if arg in self._backends]
for p, val in sorted(params.items()):
if p in self._backends:
imports.append((p, self._backends[p]))
if not imports:
imports = [('matplotlib', 'mpl')]

args = list(args)
selected_backend = None
for backend, imp in imports:
try:
__import__('holoviews.plotting.%s' % imp)
if selected_backend is None:
selected_backend = backend
Copy link
Contributor

Choose a reason for hiding this comment

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

Good to see it is now obeying the specified order, with the first thing listed activated.

except ImportError:
if backend in args:
args.pop(args.index(backend))
Expand Down Expand Up @@ -187,7 +194,7 @@ def __call__(self, *args, **params):
ip = get_ipython() if ip is None else ip # noqa (get_ipython)
param_ext.load_ipython_extension(ip, verbose=False)
load_magics(ip)
OutputMagic.initialize(list( self._backends.keys()))
OutputMagic.initialize([backend for backend, _ in imports])
set_display_hooks(ip)
notebook_extension._loaded = True

Expand All @@ -209,11 +216,10 @@ def __call__(self, *args, **params):

message = '' if not p.banner else '%s successfully loaded in this cell.' % loaded
load_hvjs(logo=p.banner, JS=('holoviews' in resources), message = message)

for r in [r for r in resources if r != 'holoviews']:
Store.renderers[r].load_nb(inline=p.inline)

if resources[-1] != 'holoviews':
get_ipython().magic(u"output backend=%r" % resources[-1]) # noqa (get_ipython))
Copy link
Contributor

Choose a reason for hiding this comment

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

Good to get rid of this..it was hackish.

Store.current_backend = selected_backend


def _get_resources(self, args, params):
Expand Down
159 changes: 99 additions & 60 deletions holoviews/ipython/magics.py
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ def get_options(cls, line, options, linemagic):
info = (keyword,value)+allowed
raise ValueError("Value %r for key %r not between %s and %s" % info)
options[keyword] = value

return cls._validate(options, items, linemagic)

@classmethod
Expand Down Expand Up @@ -231,21 +230,28 @@ class OutputMagic(OptionsMagic):
'max-width', 'min-width', 'max-height',
'min-height', 'outline', 'float']}}

defaults = OrderedDict([('backend' , 'matplotlib'),
('fig' , 'png'),
('holomap' , 'widgets'),
('widgets' , 'embed'),
('fps' , 20),
defaults = OrderedDict([('backend' , None),
('fig' , None),
('holomap' , None),
('widgets' , None),
('fps' , None),
('max_frames' , 500),
('max_branches', 2),
('size' , 100),
('dpi' , 72),
('size' , None),
('dpi' , None),
('charwidth' , 80),
('filename' , None),
('info' , False),
('css' , {})])
('css' , None)])

# Defines the options the OutputMagic remembers. All other options
# are held by the backend specific Renderer.
remembered = ['max_frames', 'max_branches', 'charwidth', 'info', 'filename']

# Remaining backend specific options renderer options
render_params = ['fig', 'holomap', 'size', 'fps', 'dpi', 'css', 'widget_mode', 'mode']

options = OrderedDict(defaults.items())
options = OrderedDict()
_backend_options = defaultdict(dict)

# Used to disable info output in testing
Expand Down Expand Up @@ -286,22 +292,23 @@ def info(cls, obj):

@classmethod
def _generate_docstring(cls):
renderer = Store.renderers[Store.current_backend]
intro = ["Magic for setting HoloViews display options.",
"Arguments are supplied as a series of keywords in any order:", '']
backend = "backend : The backend used by HoloViews %r" % cls.allowed['backend']
fig = "fig : The static figure format %r" % cls.allowed['fig']
holomap = "holomap : The display type for holomaps %r" % cls.allowed['holomap']
widgets = "widgets : The widget mode for widgets %r" % cls.allowed['widgets']
widgets = "widgets : The widget mode for widgets %r" % renderer.widget_mode
fps = ("fps : The frames per second for animations (default %r)"
% cls.defaults['widgets'])
% renderer.fps)
frames= ("max_frames : The max number of frames rendered (default %r)"
% cls.defaults['max_frames'])
branches=("max_branches : The max number of Layout branches rendered (default %r)"
% cls.defaults['max_branches'])
size = ("size : The percentage size of displayed output (default %r)"
% cls.defaults['size'])
% renderer.size)
dpi = ("dpi : The rendered dpi of the figure (default %r)"
% cls.defaults['dpi'])
% renderer.dpi)
chars = ("charwidth : The max character width for displaying the output magic (default %r)"
% cls.defaults['charwidth'])
fname = ("filename : The filename of the saved output, if any (default %r)"
Expand Down Expand Up @@ -337,25 +344,48 @@ def output(self, line, cell=None):
print("\nFor help with the %output magic, call %output?")
return

restore_copy = OrderedDict(OutputMagic.options.items())
# Make backup of previous options
prev_backend = Store.current_backend
prev_renderer = Store.renderers[prev_backend]
prev_mode = prev_renderer.mode
prev_params = {k: v for k, v in prev_renderer.get_param_values()
if k in self.render_params}
prev_restore = dict(OutputMagic.options)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe prev is confusing as it suggests the backend is changing (which it might not). Maybe initial_backend? Implement this if you like the suggestion, it isn't something worth holding up the PR for.

try:
options = OrderedDict(OutputMagic.options.items())
new_options = self.get_options(line, options, cell is None)
self._set_render_options(new_options)
# Process magic
new_options = self.get_options(line, {}, cell is None)

# Make backup of options on selected renderer
if 'backend' in new_options:
backend = new_options['backend']
if ':' not in backend:
backend += ':default'
else:
backend = prev_backend+':'+prev_mode
renderer = Store.renderers[backend.split(':')[0]]
render_params = {k: v for k, v in renderer.get_param_values()
if k in self.render_params}

# Set options on selected renderer and set display hook options
OutputMagic.options = new_options
self._set_render_options(new_options, backend)
except Exception as e:
self.update_options(options, {'backend': restore_copy['backend']})
OutputMagic.options = restore_copy
self._set_render_options(restore_copy)
# If setting options failed ensure they are reset
OutputMagic.options = prev_restore
self.set_backend(prev_backend)
print('Error: %s' % str(e))
print("For help with the %output magic, call %output?\n")
return

if cell is not None:
self.shell.run_cell(cell, store_history=STORE_HISTORY)
self.update_options(options, {'backend': restore_copy['backend']})
OutputMagic.options = restore_copy
self._set_render_options(restore_copy)
# After cell magic restore previous options and restore
# temporarily selected renderer
OutputMagic.options = prev_restore
self._set_render_options(render_params, backend)
if backend.split(':')[0] != prev_backend:
self.set_backend(prev_backend)
self._set_render_options(prev_params, prev_backend+':'+prev_mode)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm tempted to say something about context managers. Maybe for another day and another PR...



@classmethod
Expand All @@ -364,74 +394,83 @@ def update_options(cls, options, items):
Switch default options and backend if new backend
is supplied in items.
"""
backend = items.get('backend', '')
# Get previous backend
prev_backend = Store.current_backend
renderer = Store.renderers[Store.current_backend]
renderer = Store.renderers[prev_backend]
Copy link
Contributor

Choose a reason for hiding this comment

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

I would move these three line:

prev_backend = Store.current_backend
renderer = Store.renderers[prev_backend]
prev_backend += ':%s' % renderer.mode

Just after the block starting with the # Get new backend comment and before # Update allowed formats.

Seeing a method starting with 'prev_backend = Store.current_backend' just seems weird but once you've seen the code about getting the new backend it makes more sense. As far as I can tell, these variables don't seem to be referenced before that point.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, so you just want to switch Get new backend and Get previous backend blocks around?

Copy link
Contributor

@jlstevens jlstevens Apr 13, 2017

Choose a reason for hiding this comment

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

I think it is a bit more readable that way. If there is an issue swapping them around, don't bother...

prev_backend += ':%s' % renderer.mode

available = backend in Store.renderers.keys()
if (not backend) or (not available) or backend == prev_backend:
# Get new backend
backend = items.get('backend', Store.current_backend)
split = backend.split(':')
core_backend, mode = split if len(split)==2 else (split[0], 'default')
Copy link
Contributor

Choose a reason for hiding this comment

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

It would be nice if we had a word that meant 'backend plus mode' so that 'backend' is the thing without the mode.

Copy link
Contributor

Choose a reason for hiding this comment

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

Then we could get rid of 'core_backend' as a name (which I don't like). Not sure what terminology would make more sense. Maybe backend+mode is a backend_spec?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure.

if ':' not in backend:
backend += ':default'

# Update allowed formats
for p in ['fig', 'holomap']:
cls.allowed[p] = list_formats(p, backend)

# Return if backend invalid and let validation error
if core_backend not in Store.renderers:
options['backend'] = core_backend
return options

cls._backend_options[prev_backend] = cls.options
# Get backend specific options
backend_options = dict(cls._backend_options[backend])
cls._backend_options[prev_backend] = {k: v for k, v in cls.options.items()
if k in cls.remembered}

# Fill in remembered options with defaults
for opt in cls.remembered:
if opt not in backend_options:
backend_options[opt] = cls.defaults[opt]

backend_options = cls._backend_options[backend]
# Switch format if mode does not allow it
for p in ['fig', 'holomap']:
opts = list_formats(p, backend)
cls.allowed[p] = opts
cls.defaults[p] = opts[0]
if p not in backend_options:
backend_options[p] = opts[0]

backend = backend.split(':')[0]
render_params = ['fig', 'holomap', 'size', 'fps', 'dpi', 'css']
for p in render_params:
if p in backend_options:
opt = backend_options[p]
cls.defaults[p] = opt
else:
opt = cls.defaults[p]
backend_options[p] = opt
if backend_options.get(p) not in cls.allowed[p]:
backend_options[p] = cls.allowed[p][0]

for opt in options:
if opt not in backend_options:
backend_options[opt] = options[opt]
# Ensure backend and mode are set
backend_options['mode'] = mode
backend_options['backend'] = backend

cls.set_backend(backend)
return backend_options


@classmethod
def initialize(cls, backend_list):
cls.backend_list = backend_list
backend = cls.options.get('backend', cls.defaults['backend'])
backend = cls.options.get('backend', Store.current_backend)
if backend in Store.renderers:
cls.options = dict(cls.defaults)
cls._set_render_options(cls.defaults)
cls.options = dict({k: cls.defaults[k] for k in cls.remembered})
cls.set_backend(backend)
else:
cls.options['backend'] = None
cls.defaults['backend'] = None
cls.set_backend(None)


@classmethod
def set_backend(cls, backend):
cls.last_backend = Store.current_backend
Store.current_backend = backend


@classmethod
def _set_render_options(cls, options):
def _set_render_options(cls, options, backend=None):
"""
Set options on current Renderer.
"""
split = options['backend'].split(':')
backend, mode = split if len(split)==2 else (split[0], 'default')
if backend:
backend = backend.split(':')[0]
else:
backend = Store.current_backend

cls.set_backend(backend)
if 'widgets' in options:
options['widget_mode'] = options['widgets']
renderer = Store.renderers[backend]
render_params = ['fig', 'holomap', 'size', 'fps', 'dpi', 'css']
render_options = {k: options[k] for k in render_params}
renderer.set_param(**dict(render_options, widget_mode=options['widgets'],
mode=mode))
render_options = {k: options[k] for k in cls.render_params if k in options}
renderer.set_param(**render_options)


@magics_class
Expand Down
3 changes: 3 additions & 0 deletions holoviews/plotting/mpl/renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ class MPLRenderer(Renderer):

backend = param.String('matplotlib', doc="The backend name.")

dpi=param.Integer(72, doc="""
The render resolution in dpi (dots per inch)""")

fig = param.ObjectSelector(default='auto',
objects=['png', 'svg', 'pdf', 'html', None, 'auto'], doc="""
Output render format for static figures. If None, no figure
Expand Down
2 changes: 1 addition & 1 deletion holoviews/plotting/renderer.py
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class Renderer(Exporter):
The full, lowercase name of the rendering backend or third
part plotting package used e.g 'matplotlib' or 'cairo'.""")

dpi=param.Integer(None, allow_None=True, doc="""
dpi=param.Integer(None, doc="""
The render resolution in dpi (dots per inch)""")

fig = param.ObjectSelector(default='auto', objects=['auto'], doc="""
Expand Down
3 changes: 1 addition & 2 deletions tests/testmagics.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,6 @@ def test_cell_opts_norm(self):
class TestOutputMagic(ExtensionTestCase):

def tearDown(self):
ipython.OutputMagic.options = ipython.OutputMagic.defaults
super(TestOutputMagic, self).tearDown()

def test_output_svg(self):
Expand Down Expand Up @@ -126,7 +125,7 @@ def test_output_size(self):

def test_output_invalid_size(self):
self.line_magic('output', "size=-50")
self.assertEqual(ipython.OutputMagic.options.get('size', None), 100)
self.assertEqual(ipython.OutputMagic.options.get('size', None), None)


class TestCompositorMagic(ExtensionTestCase):
Expand Down