-
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Test fixes 2 #994
Test fixes 2 #994
Changes from 27 commits
3cef6d3
ce3b723
7378270
fbdcb62
39d036d
53bcdec
c88ef40
75b0ffc
c2821a6
f7c75cc
c228895
22f15c0
57a56aa
0fc2727
6f8dcf4
20ecb25
f024f57
47622f5
390c4f2
6d5dd92
f70202d
6455837
0fedc1b
6ed70db
c891d75
77ca1b2
f09a5ec
0b5a98c
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 |
---|---|---|
|
@@ -31,6 +31,7 @@ | |
from plotly.api import v1, v2 | ||
from plotly.basedatatypes import BaseTraceType | ||
from plotly.plotly import chunked_requests | ||
from plotly.graph_objs import Scatter | ||
from plotly.grid_objs import Grid, Column | ||
from plotly.dashboard_objs import dashboard_objs as dashboard | ||
|
||
|
@@ -304,7 +305,6 @@ def plot_mpl(fig, resize=True, strip_style=False, update=None, **plot_options): | |
fig = tools.mpl_to_plotly(fig, resize=resize, strip_style=strip_style) | ||
if update and isinstance(update, dict): | ||
fig.update(update) | ||
fig.validate() | ||
elif update is not None: | ||
raise exceptions.PlotlyGraphObjectError( | ||
"'update' must be dictionary-like and a valid plotly Figure " | ||
|
@@ -588,7 +588,7 @@ def open(self): | |
streaming_specs = self.get_streaming_specs() | ||
self._stream = chunked_requests.Stream(**streaming_specs) | ||
|
||
def write(self, trace, layout=None, validate=True, | ||
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. removed validate so that users will not be able to use the internal validation |
||
def write(self, trace, layout=None, | ||
reconnect_on=(200, '', 408)): | ||
""" | ||
Write to an open stream. | ||
|
@@ -606,9 +606,6 @@ def write(self, trace, layout=None, validate=True, | |
keyword arguments: | ||
layout (default=None) - A valid Layout object | ||
Run help(plotly.graph_objs.Layout) | ||
validate (default = True) - Validate this stream before sending? | ||
This will catch local errors if set to | ||
True. | ||
|
||
Some valid keys for trace dictionaries: | ||
'x', 'y', 'text', 'z', 'marker', 'line' | ||
|
@@ -629,15 +626,24 @@ def write(self, trace, layout=None, validate=True, | |
http://nbviewer.ipython.org/github/plotly/python-user-guide/blob/master/s7_streaming/s7_streaming.ipynb | ||
|
||
""" | ||
# always bypass validation in here as | ||
# now automatically done | ||
validate = False | ||
|
||
# Convert trace objects to dictionaries | ||
if isinstance(trace, BaseTraceType): | ||
trace = tracefill_percent | ||
trace = trace.to_plotly_json() | ||
|
||
stream_object = dict() | ||
stream_object.update(trace) | ||
if 'type' not in stream_object: | ||
# tests if Scatter contains invalid kwargs | ||
dummy_obj = copy.deepcopy(Scatter(**stream_object)) | ||
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. Here I am making a deepcopy of the trace being coerced into a Scatter from the original dict. The point is that an error will pop up if there is one, and stream_object can continue down the code being untouched and do its thing. Do you think this is a good idea? Is it better to have the actual object the user sent raise the error? |
||
stream_object = Scatter(**stream_object) | ||
stream_object['type'] = 'scatter' | ||
|
||
# TODO: remove this validation as now it's | ||
# done automatically | ||
if validate: | ||
try: | ||
tools.validate(stream_object, stream_object['type']) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,5 @@ | ||
import sys | ||
from unittest import TestCase | ||
from nose.tools import raises | ||
|
||
import plotly.graph_objs as go | ||
|
||
|
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.
does it create an issue to create a dependancy of
graph_objs
forplotly.py
?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.
I don't think so. There shouldn't be an existing dependency the other way (i.e.
plotly.graph_objs
->plotly.plotly
)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.
That's good. I only brought it up because another engineer advised against doing that i.e. sent me an blogpost saying it could be a problem