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

Conform iplot_histogram and plot_histogram #866

Merged
merged 6 commits into from
Sep 7, 2018
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion qiskit/tools/visualization/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,14 +11,18 @@

from ._circuit_visualization import circuit_drawer, plot_circuit, generate_latex_source,\
latex_circuit_drawer, matplotlib_circuit_drawer, qx_color_scheme
from ._counts_visualization import plot_histogram

if ('ipykernel' in sys.modules) and ('spyder' not in sys.modules):
import requests
if requests.get(
'https://qvisualization.mybluemix.net/').status_code == 200:
from .interactive._iplot_state import iplot_state as plot_state
from .interactive._iplot_histogram import iplot_histogram as \
plot_histogram
else:
from ._state_visualization import plot_state
from ._counts_visualization import plot_histogram

else:
from ._state_visualization import plot_state
from ._counts_visualization import plot_histogram
87 changes: 60 additions & 27 deletions qiskit/tools/visualization/_counts_visualization.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,35 +17,68 @@
import matplotlib.pyplot as plt


def plot_histogram(data, number_to_keep=False):
def plot_histogram(data, number_to_keep=False, legend=None, options=None):
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we emit deprecation warnings for number_to_keep? Change the signature of the function to:

def plot_histogram(data, legend=None, options=None, **kwargs):

And check if number_to_keep has been passed. If so, emit a deprecation warning:

if 'numbers_to_keep' in kwargs:
  warnings.warn('number_to_keep has been deprecated, use ...' DeprecationWarning)

numbers_to_keep = kwargs.get('numbers_to_keep', False)

Copy link
Member Author

Choose a reason for hiding this comment

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

I don't want to change the signature here, because the order of named kwargs actually matters. For example someone could be using this like: plot_histogram(data_input, 5) and that would work fine but if I move numbers_to_keep those users will be broken. But, I'll add the deprecation warning

"""Plot a histogram of data.

data is a dictionary of {'000': 5, '010': 113, ...}
number_to_keep is the number of terms to plot and rest is made into a
single bar called other values
Args:
data (list or dict): This is either a list of dictionaries containing:
values to represent (ex {'001': 130})
Copy link
Member

Choose a reason for hiding this comment

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

can you complete this sentence: "either ... or"?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

number_to_keep (int): DEPRECATED the number of terms to plot and rest
is made into a single bar called other values
legend(list): A list of strings to use for labels of the data.
The number of entries must match the lenght of data (if data is a
list or 1 if it's a dict)
options (dict): Representation settings containing
- number_to_keep (integer): groups max values
- show_legend (bool): show legend of graph content
Raises:
Exception: When legend is provided and the length doesn't match the
input data
"""
if number_to_keep is not False:
data_temp = dict(Counter(data).most_common(number_to_keep))
data_temp["rest"] = sum(data.values()) - sum(data_temp.values())
data = data_temp

labels = sorted(data)
values = np.array([data[key] for key in labels], dtype=float)
pvalues = values / sum(values)
numelem = len(values)
ind = np.arange(numelem) # the x locations for the groups
width = 0.35 # the width of the bars
if options is None:
options = {}

if isinstance(data, dict):
data = [data]
if legend and len(legend) != 1:
raise Exception("Length of legendL %s doesn't match number of "
Copy link
Contributor

Choose a reason for hiding this comment

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

We try to use functionality-specific errors, subclassing QiskitError. I think a VisualizationError class would make sense here. Do you mind to create this new class of errors and pass through other raise statements and change them accordingly?

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

"input executions: %s" % (len(legend), 1))
else:
if legend and len(legend) != len(data):
raise Exception("Length of legendL %s doesn't match number of "
"input executions: %s" % (len(legend), len(data)))

_, ax = plt.subplots()
rects = ax.bar(ind, pvalues, width, color='seagreen')
# add some text for labels, title, and axes ticks
ax.set_ylabel('Probabilities', fontsize=12)
ax.set_xticks(ind)
ax.set_xticklabels(labels, fontsize=12, rotation=70)
ax.set_ylim([0., min([1.2, max([1.2 * val for val in pvalues])])])
# attach some text labels
for rect in rects:
height = rect.get_height()
ax.text(rect.get_x() + rect.get_width() / 2., 1.05 * height,
'%f' % float(height),
ha='center', va='bottom')
for item, execution in enumerate(data):
Copy link
Contributor

Choose a reason for hiding this comment

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

Non-blocking: do you think is it feasible or even useful to allow customization of graph width, graph height and sort to equate the options dictionary passed to this function to that supported by the iplot_function?

Copy link
Member Author

Choose a reason for hiding this comment

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

It should be, and I thought about that when I was refactoring things. But I didn't want to change things too much here. After this merges I'll work on updating this to add the missing options.

if number_to_keep is not False or (
'number_to_keep' in options and options['number_to_keep']):
data_temp = dict(Counter(execution).most_common(number_to_keep))
data_temp["rest"] = sum(execution.values()) - sum(data_temp.values())
execution = data_temp

labels = sorted(execution)
values = np.array([execution[key] for key in labels], dtype=float)
pvalues = values / sum(values)
numelem = len(values)
ind = np.arange(numelem) # the x locations for the groups
width = 0.35 # the width of the bars
label = None
if legend:
label = legend[item]
adj = width * item
rects = ax.bar(ind+adj, pvalues, width, label=label)
# add some text for labels, title, and axes ticks
ax.set_ylabel('Probabilities', fontsize=12)
ax.set_xticks(ind)
ax.set_xticklabels(labels, fontsize=12, rotation=70)
ax.set_ylim([0., min([1.2, max([1.2 * val for val in pvalues])])])
# attach some text labels
for rect in rects:
height = rect.get_height()
ax.text(rect.get_x() + rect.get_width() / 2., 1.05 * height,
'%f' % float(height),
ha='center', va='bottom')
if legend and (
'show_legend' not in options or options['show_legend'] is True):
plt.legend()
plt.show()
39 changes: 30 additions & 9 deletions qiskit/tools/visualization/interactive/_iplot_histogram.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,24 +48,29 @@ def process_data(data, number_to_keep):
return result


def iplot_histogram(executions_results, options=None):
def iplot_histogram(data, number_to_keep=False, options=None, legend=None):
""" Create a histogram representation.

Graphical representation of the input array using a vertical bars
style graph.

Args:
executions_results (array): Array of dictionaries containing
- data (dict): values to represent (ex. {'001' : 130})
- name (string): name to show in the legend
- device (string): Could be 'real' or 'simulated'
data (list or dict): This is either a list of dicts or a single
dict containing the values to represent (ex. {'001' : 130})
number_to_keep (int): DEPRECATED the number of terms to plot and
rest is made into a single bar called other values
legend (list): A list of strings to use for labels of the data.
The number of entries must match the length of data.
options (dict): Representation settings containing
- width (integer): graph horizontal size
- height (integer): graph vertical size
- slider (bool): activate slider
- number_to_keep (integer): groups max values
- show_legend (bool): show legend of graph content
- sort (string): Could be 'asc' or 'desc'
Raises:
Exception: When legend is provided and the length doesn't match the
input data
"""

# HTML
Expand Down Expand Up @@ -111,12 +116,28 @@ def iplot_histogram(executions_results, options=None):
options['show_legend'] = 1

if 'number_to_keep' not in options:
options['number_to_keep'] = 0
if number_to_keep is False:
options['number_to_keep'] = 0
elif number_to_keep:
options['number_to_keep'] = number_to_keep

data_to_plot = []
for execution in executions_results:
data = process_data(execution['data'], options['number_to_keep'])
data_to_plot.append({'data': data})
if isinstance(data, dict):
data = [data]
if legend and len(legend) != 1:
raise Exception("Length of legendL %s doesn't match number of "
"input executions: %s" % (len(legend), 1))
else:
if legend and len(legend) != len(data):
raise Exception("Length of legendL %s doesn't match number of "
"input executions: %s" % (len(legend), len(data)))

for item, execution in enumerate(data):
exec_data = process_data(execution, options['number_to_keep'])
out_dict = {'data': exec_data}
if legend:
out_dict['name'] = legend[item]
data_to_plot.append(out_dict)

html = html_template.substitute({
'divNumber': div_number
Expand Down