-
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Distplot #275
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
Conversation
- Plotly version of seaborn.distplot to add to FigureFactory - plots histogram, curve (kde or normal), and rugplot
- add `validate_displot()` method to tools.py - add core tests for distplot parameters
- add protected import of scipy to tools.py - add distplot checks to tests_optional
- add scipy.stats - edit examples in docstring
- all distplot tests are depending on scipy
@chriddyp @theengineear @aneda @etpinard
Hey everyone, here's a draft of the distplot (http://stanford.edu/~mwaskom/software/seaborn/generated/seaborn.distplot.html) FigureFactory addition.
One issue though - I'm not sure if the 'protected import' of scipy I tried is how we want to add it / correct in general- the tests won't pass on circle.
@cldougl from the Circle error:
======================================================================
ERROR: test_distplot_more_args (plotly.tests.test_optional.test_opt_tracefactory.TestDistplot)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/home/ubuntu/python-api/plotly/tests/test_optional/test_opt_tracefactory.py", line 109, in test_distplot_more_args
show_rug=False, bin_size=.2)
File "/home/ubuntu/python-api/plotly/tools.py", line 2418, in create_distplot
FigureFactory.validate_distplot(hist_data, curve_type)
File "/home/ubuntu/python-api/plotly/tools.py", line 1541, in validate_distplot
raise ImportError("FigureFactory.create_distplot requires scipy")
ImportError: FigureFactory.create_distplot requires scipy
it looks like our Circle build doesn't have scipy. Circle will only build the packages that we explicitly configure. Here is the circle configuration file: https://github.com/plotly/python-api/blob/master/circle.yml#L7, note the "optional-requirements.txt" reference. if you add scipy to that file: https://github.com/plotly/python-api/blob/master/optional-requirements.txt, then circle should build it and tests should pass
ahh thank you @chriddyp !!
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.
\o/
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.
Note that this is going to be a merge conflict with #274 . I guess we'll need to have some specific requirements specified... How about a syntax like:
_scipy_imported
_scipy__spatial_imported
_scipy__cluster__hierarchy_imported
I'm not sure which is going to land first, but the latter will have to implement this.
- DRYing type validation - fix over-indentation
plotly/tools.py
Outdated
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.
pep:8ball: spaces around operators
plotly/tools.py
Outdated
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.
can we make these guys hidden? (by adding a _). there's hardly anything I like more than seeing the minimal number of functions in the ipython tab-tab helper:
image
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.
Could we reverse the legend? That way way the rugs will be displayed in the same order as the legend entries. Right now they're in opposite orders
image
otherwise, sweet!
- hide validate functions from tab-tab helper - add pandas example and pandas type to validation - replace `**kwargs` with `rug_text` and `colors` in - updated the docstring with Andrew’s nice terse suggestions - hid tick label from rug plot - modified rug_text so its indexed across datasets, not copied - edited examples - added import numpy where needed - increased sample sizes - reversed legend - fix the show_legend issue so a legend is shown if the hist is hidden - add test to test_core to check exception for `scipy_import = False` - edit tests in test optional to reflect the changes (mentioned above) made
- scipy will currently import in test_core
@chriddyp can I merge this?
Looking good! It doesn't look like the colors argument is working for me:
import plotly.plotly as py
from plotly.tools import FigureFactory as FF
import numpy as np
x1 = np.random.randn(200)
x2 = np.random.randn(200)+1
x3 = np.random.randn(200)-1
x4 = np.random.randn(200)+2
hist_data = [x1] + [x2] + [x3] + [x4]
group_labels = ['2012', '2013', '2014', '2015']
fig = FF.create_distplot(
hist_data, group_labels, curve_type='normal',
bin_size=.2, colors=['yellow', 'pink', 'grey', 'aqua'])
iplot(fig)
Thx for fixing! All good by me now 💃 Can you add some python examples as ipython notebooks to /documentation?
Thanks! yep doc's ready
validate_displot()method to tools.py