-
Notifications
You must be signed in to change notification settings - Fork 16
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
Decouple from matplotlib and add Bokeh for backend #57
base: master
Are you sure you want to change the base?
Changes from 2 commits
8849090
0ea4204
69feff7
77e7491
b563216
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -7,6 +7,9 @@ | |
import matplotlib.pyplot as plt | ||
import numpy as np | ||
|
||
from .backends.mtpltlib import Matplotlib | ||
from .backends.bkh import Bokeh | ||
|
||
|
||
class GridStrategy(metaclass=ABCMeta): | ||
""" | ||
|
@@ -15,8 +18,38 @@ class GridStrategy(metaclass=ABCMeta): | |
nearly square (nearly equal in both dimensions). | ||
""" | ||
|
||
def __init__(self, alignment="center"): | ||
def __init__(self, alignment="center", backend="matplotlib"): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think instead of taking a string that says what the backend is, we should define an abstract base class and/or a Protocol and take that. This will allow anyone to write their own backends for it without needing to submit them upstream. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Also, I notice that the backend objects take "alignment" and so does GridStrategy, which seems wrong, no? I think if anything the alignment should be passed to the relevant backend objects' functions. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was under the impression that Python didn't have this kind of static typing stuff. This is pretty cool. I'll look into it if I have time. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You don't have to worry too much about the protocol stuff. Make it an ABC to start with to organizer your thinking - the point is to advertise exactly what parts of the interface are public. |
||
self.alignment = alignment | ||
self.supported_backends = ["matplotlib", "bokeh"] | ||
self.library = None | ||
|
||
assert ( | ||
backend in self.supported_backends | ||
), f"Library {backend} is not a supported backend." | ||
if backend == "matplotlib": | ||
try: | ||
import matplotlib | ||
except ImportError: | ||
print( | ||
"matplotlib not installed. Please install it to use it with grid_strategy." | ||
) | ||
self.library = Matplotlib(alignment=self.alignment) | ||
|
||
elif backend == "bokeh": | ||
try: | ||
import bokeh | ||
except ImportError: | ||
print( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You should raise your own Considering my other suggestion, to take a DEFAULT_BACKEND = None
try:
from .matplotlib_backend import MatplotlibBackend
DEFAULT_BACKEND = MatplotlibBackend
except ImportError:
pass
if DEFAULT_BACKEND is None:
try:
from .bokeh_backend import BokehBackend
DEFAULT_BACKEND = BokehBackend
except ImportError:
pass
if DEFAULT_BACKEND is None:
raise ImportError("Could not find a suitable backend - please install either matplotlib or bokeh") Then have the constructor take backend=DEFAULT_BACKEND. In the future we can rework this with |
||
"Bokeh is not installed. Please install it to use it with grid_strategy." | ||
) | ||
self.library = Bokeh(alignment=self.alignment) | ||
|
||
# elif backend == "plotly": | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I generally recommend against checking in commented-out code, we can move this to another branch or a later commit, no? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, i didnt want to get rid of it so I didn't forget how I implemented it before hand. It had a bug that was difficult to understand so I just dropped it to implement bokeh instead. |
||
# try: | ||
# import plotly | ||
# except ImportError: | ||
# print("plotly not installed. Please install it to use it with grid_strategy.") | ||
# self.library = Plotly(alignment=self.alignment) | ||
|
||
def get_grid(self, n): | ||
""" | ||
|
@@ -30,70 +63,22 @@ def get_grid(self, n): | |
where each x would be a subplot. | ||
""" | ||
|
||
if n < 0: | ||
raise ValueError | ||
grid_arrangement = self.get_grid_arrangement(n) | ||
return self.get_gridspec(grid_arrangement) | ||
return self.get_figures(grid_arrangement) | ||
|
||
@classmethod | ||
@abstractmethod | ||
def get_grid_arrangement(cls, n): # pragma: nocover | ||
pass | ||
|
||
def get_gridspec(self, grid_arrangement): | ||
def get_figures(self, grid_arrangement): | ||
nrows = len(grid_arrangement) | ||
ncols = max(grid_arrangement) | ||
|
||
# If it has justified alignment, will not be the same as the other alignments | ||
if self.alignment == "justified": | ||
return self._justified(nrows, grid_arrangement) | ||
else: | ||
return self._ragged(nrows, ncols, grid_arrangement) | ||
|
||
def _justified(self, nrows, grid_arrangement): | ||
ax_specs = [] | ||
num_small_cols = np.lcm.reduce(grid_arrangement) | ||
gs = gridspec.GridSpec( | ||
nrows, num_small_cols, figure=plt.figure(constrained_layout=True) | ||
) | ||
for r, row_cols in enumerate(grid_arrangement): | ||
skip = num_small_cols // row_cols | ||
for col in range(row_cols): | ||
s = col * skip | ||
e = s + skip | ||
|
||
ax_specs.append(gs[r, s:e]) | ||
return ax_specs | ||
|
||
def _ragged(self, nrows, ncols, grid_arrangement): | ||
if len(set(grid_arrangement)) > 1: | ||
col_width = 2 | ||
return self.library._justified(nrows, grid_arrangement) | ||
else: | ||
col_width = 1 | ||
|
||
gs = gridspec.GridSpec( | ||
nrows, ncols * col_width, figure=plt.figure(constrained_layout=True) | ||
) | ||
|
||
ax_specs = [] | ||
for r, row_cols in enumerate(grid_arrangement): | ||
# This is the number of missing columns in this row. If some rows | ||
# are a different width than others, the column width is 2 so every | ||
# column skipped at the beginning is also a missing slot at the end. | ||
if self.alignment == "left": | ||
# This is left-justified (or possibly full justification) | ||
# so no need to skip anything | ||
skip = 0 | ||
elif self.alignment == "right": | ||
# Skip two slots for every missing plot - right justified. | ||
skip = (ncols - row_cols) * 2 | ||
else: | ||
# Defaults to centered, as that is the default value for the class. | ||
# Skip one for each missing column - centered | ||
skip = ncols - row_cols | ||
|
||
for col in range(row_cols): | ||
s = skip + col * col_width | ||
e = s + col_width | ||
|
||
ax_specs.append(gs[r, s:e]) | ||
|
||
return ax_specs | ||
return self.library._ragged(nrows, ncols, grid_arrangement) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
from bokeh.layouts import layout, Spacer | ||
from bokeh.io import show, output_file | ||
|
||
|
||
class JustifiedGrid: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. These two seem to share a decent amount of code, and I'm assuming they entirely share their substantive interface - maybe refactor into a |
||
def __init__(self, nrows, grid_arrangement): | ||
self.plots = [[]] | ||
self.grid_arrangement = grid_arrangement | ||
self.current_row = 0 | ||
self.nrows = nrows | ||
|
||
def add_plot(self, plot): | ||
self.plots[self.current_row].append(plot) | ||
if len(self.plots[self.current_row]) == self.grid_arrangement[self.current_row]: | ||
self.plots.append([]) | ||
self.current_row += 1 | ||
assert ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just to clarify - you should not use assertions for exception handling. When to use assertions is a tricky business, but basically think that assertion code won't be run at all in some circumstances (if you run Not quite sure what category this fits in. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll switch it to a try except block I think, since I think it would be fair to consider that user's requesting for n grids and then trying to insert more than n figures would be considered an error. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
self.current_row <= self.nrows | ||
), "Error: More graphs added to layout than previously specified." | ||
|
||
def add_plots(self, plot_list): | ||
for plot in plot_list: | ||
self.add_plot(plot) | ||
|
||
def output_dest(self, file): | ||
output_file(file) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't know bokeh - were you supposed to return this, or is it the side effect you care about? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bokeh creates html files as it's output medium, so that function doesn't return anything useful. I just sets the output file. |
||
|
||
def show_plot(self): | ||
l = layout(self.plots, sizing_mode="stretch_width") | ||
show(l) | ||
|
||
|
||
class AlignedGrid: | ||
def __init__(self, nrows, ncols, grid_arrangement, alignment): | ||
self.plots = [[]] | ||
self.grid_arrangement = grid_arrangement | ||
self.alignment = alignment | ||
self.current_row = 0 | ||
self.nrows = nrows | ||
|
||
def add_plot(self, plot): | ||
self.plots[self.current_row].append(plot) | ||
if len(self.plots[self.current_row]) == self.grid_arrangement[self.current_row]: | ||
self.plots.append([]) | ||
self.current_row += 1 | ||
assert ( | ||
self.current_row <= self.nrows | ||
), "Error: More graphs added to the layout than previously specified." | ||
|
||
def add_plots(self, plots): | ||
for plot in plots: | ||
self.add_plot(plot) | ||
|
||
def output_dest(self, file): | ||
output_file(file) | ||
|
||
def show_plot(self): | ||
for row in self.plots: | ||
if len(row) == max(self.grid_arrangement): | ||
continue | ||
else: | ||
if self.alignment == "left": | ||
row.append(Spacer()) | ||
elif self.alignment == "right": | ||
row.insert(0, Spacer()) | ||
elif self.alignment == "center": | ||
row.append(Spacer()) | ||
row.insert(0, Spacer()) | ||
l = layout(self.plots, sizing_mode="scale_both") | ||
show(l) | ||
|
||
|
||
class Bokeh: | ||
def __init__(self, alignment): | ||
self.alignment = alignment | ||
|
||
def _justified(self, nrows, grid_arrangement): | ||
return JustifiedGrid(nrows, grid_arrangement) | ||
|
||
def _ragged(self, nrows, ncols, grid_arrangement): | ||
return AlignedGrid(nrows, ncols, grid_arrangement, self.alignment) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
from matplotlib import gridspec | ||
import matplotlib.pyplot as plt | ||
import numpy as np | ||
|
||
|
||
class Matplotlib: | ||
def __init__(self, alignment="center"): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably something for a future change, but I do think it would be nice to be able to pass a pre-existing figure to be populated by the backend. I think I didn't see a great way to do that without doubling down on the tight coupling to |
||
self.alignment = alignment | ||
|
||
def _justified(self, nrows, grid_arrangement): | ||
ax_specs = [] | ||
num_small_cols = np.lcm.reduce(grid_arrangement) | ||
gs = gridspec.GridSpec( | ||
nrows, num_small_cols, figure=plt.figure(constrained_layout=True) | ||
) | ||
for r, row_cols in enumerate(grid_arrangement): | ||
skip = num_small_cols // row_cols | ||
for col in range(row_cols): | ||
s = col * skip | ||
e = s + skip | ||
|
||
ax_specs.append(gs[r, s:e]) | ||
return ax_specs | ||
|
||
def _ragged(self, nrows, ncols, grid_arrangement): | ||
if len(set(grid_arrangement)) > 1: | ||
col_width = 2 | ||
else: | ||
col_width = 1 | ||
|
||
gs = gridspec.GridSpec( | ||
nrows, ncols * col_width, figure=plt.figure(constrained_layout=True) | ||
) | ||
|
||
ax_specs = [] | ||
for r, row_cols in enumerate(grid_arrangement): | ||
# This is the number of missing columns in this row. If some rows | ||
# are a different width than others, the column width is 2 so every | ||
# column skipped at the beginning is also a missing slot at the end. | ||
if self.alignment == "left": | ||
# This is left-justified (or possibly full justification) | ||
# so no need to skip anything | ||
skip = 0 | ||
elif self.alignment == "right": | ||
# Skip two slots for every missing plot - right justified. | ||
skip = (ncols - row_cols) * 2 | ||
else: | ||
# Defaults to centered, as that is the default value for the class. | ||
# Skip one for each missing column - centered | ||
skip = ncols - row_cols | ||
|
||
for col in range(row_cols): | ||
s = skip + col * col_width | ||
e = s + col_width | ||
|
||
ax_specs.append(gs[r, s:e]) | ||
|
||
return ax_specs | ||
|
||
|
||
# class Plotly: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this all commented out? Does it not work yet? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as above, I will work on it later. |
||
# from plotly.subplots import make_subplots | ||
# import numpy as np | ||
|
||
# def __init__(self, alignment="center"): | ||
# self.alignment = alignment | ||
|
||
# def _justified(self, nrows, grid_arrangement): | ||
# num_small_cols = int(self.np.lcm.reduce(grid_arrangement)) | ||
|
||
# specs = [] | ||
|
||
# for row_cols in grid_arrangement: | ||
# width = num_small_cols // row_cols | ||
|
||
# row = [] | ||
# for _ in range(row_cols): | ||
# row.append({"colspan":width}) | ||
# row.extend([None]*(width-1)) | ||
# specs.append(row) | ||
|
||
# fig = self.make_subplots( | ||
# nrows, | ||
# num_small_cols, | ||
# specs = specs | ||
# ) | ||
# return fig, grid_arrangement | ||
|
||
# def _ragged(self, nrows, ncols, grid_arrangement): | ||
# pass |
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.
LGTM bot thing is complaining that these imports are only necessary for the matplotlib backend.