Skip to content

Adds documentation and violin plots!#107

Open
jpoles1 wants to merge 10 commits into
santosjorge:masterfrom
jpoles1:master
Open

Adds documentation and violin plots!#107
jpoles1 wants to merge 10 commits into
santosjorge:masterfrom
jpoles1:master

Conversation

@jpoles1

@jpoles1 jpoles1 commented Apr 15, 2018

Copy link
Copy Markdown

I think the documentation is the more significant contribution here. Fixes #104.

That said, I do like violin plots and use them often in my research. They are added here using figure_factory. I hope that this code is up to snuff and is reasonably in line with the style/organization of the rest of the codebase. Happy to tune this up further however you'd like @santosjorge. Fixes #50.

Normally I'd try and keep these two contributions separate, but I wanted to use the documentation in order to better develop the violin plots, so I ended up working on them at the same time, entangling their commits. That's my bad.

@santosjorge

Copy link
Copy Markdown
Owner

Thank you @jpoles1 for this. It seems there is a lot going on on this PR.

I see what you did with the violin charts, and is a problem I ran with before with the old candle charts (which were generated by the figure_factory before). These methods should not be called directly from iplot as it breaks the usage of the other arguments, as well as being able to switch to different themes.

Either these methods need to be called separately (as df.scatter_matrix does today) and not inside iplot, or alternatively the violin chart needs to be rewritten inside iplot so it can take into account the rest of the arguments.

That is why I previously suggested that I was going to give it a go with the distplot chart (only that I am on holidays, hence the delay). However happy if you want to point other people to your fork for this solution meanwhile.

Regarding the documentation - thank you for this. I was working with some guys that were developing a new doc generator that looked very promising, but I may go with Sphinx until then. Let me check the doc implementation you have here next week - and perhaps we can extract only that commit from this PR.

@IvoMerchiers

Copy link
Copy Markdown

@santosjorge any update on this? Because I think cufflinks would benefit greatly from proper documentation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Detailed Code Documentation Violin plot ?

3 participants