Adding Scatterplot Matrix to FigureFactory #417
Conversation
| def create_scatterplotmatrix(df, useindex = False, index = " ", | ||
| diagonal = "Scatter", symbol = 0, size = 6, | ||
| height = " ", width = " ", jitter = 0, | ||
| title = "Scatterplot Matrix"): |
There was a problem hiding this comment.
We typically keep pretty lengthy docs for things like this
This way, when users run help, the information is right there.
|
@Kully thanks for submitting this! The logic flow is super clear. I do have some general concerns:
Let me know when you want another round of review, happy to cruise through again. Thanks for the awesome work so far! |
|
@theengineear Thank you so much for the feedback. I would definitely want some more feedback in another round of review. |
|
@Kully please feel free to reach out with any questions Git, Python, otherwise! |
|
Looking good! I'd love to see an optional Here are some nice examples.
|
|
@Kully Awesome! I'm a little swamped until Tuesday, I probably won't be able to give this a good look until then. Feel free to give me a poke if I don't respond by Tuesday night though! I don't want this PR to loose steam! |
|
@Kully awesome -- I'll take a look at this tonight as well!! |
|
@Kully Taking a look now. |
|
@Kully The tests are failing because you need to update the default schema. The short story is that we force new merges to keep
in your terminal to get tests to pass again. Let me know if you need any help with that! |
| diagonal = "Scatter", symbol = 0, size = 6, | ||
| height = " ", width = " ", jitter = 0, | ||
| title = "Scatterplot Matrix"): | ||
| def _scatterplot_no_index(matrix, headers, |
There was a problem hiding this comment.
🐄 , any reason you didn't use the full 79 characters of line length here?
|
💃💃💃 when tests pass |
|
Ditto: 👯 after tests! |
| } | ||
|
|
||
| self.maxDiff = None | ||
| self.assertEqual(test_scatter_plot_matrix.data, |
There was a problem hiding this comment.
@Kully looks like you left .data appended to test_scatter_plot_matrix
|
@cldougl Just removed the .data |
|
@Kully and @cldougl let me know if you'd like any help debugging test failures. One quick tip to checkout would be the 2-->3 gotcha with Have you been able to run the Python 3 test suite locally successfully? |
|
@theengineear We tried 3.4 and it was working locally. But we haven't tried 3.3 yet. |
|
@cldougl @theengineear I changed the diag param from 'histogram' to both 'scatter' and 'box' (and the respective expected scatter plot matrices) and it worked! Will check out 'histogram' option again and see if it fails. |
|
@cldougl @theengineear @jackparmer Good and ready for a merge! |
|
Not a huge deal, but in the future, it's helpful for reviewing when the commit history is clean:
I'm guessing those commits were related to failures only occurring on Circle? Even if that's the case, you can rewrite git history (as long as no one is branching off your branch) with That's a little advanced Again, just in the future 😸. Don't worry about it now. |
|
💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 💃 |
@cldougl
This is the first PR for the Scatterplot Matrix in the FigureFactory class in tools.py. The function currently takes in only pandas dataframes, does sanity checks on the dataframe (more than one column, either strings or numbers within a column, etc.) and then builds a scatterplot matrix based on this input. There is an option for indexing by one of the columns which must be inputted by the user (parameter: index) and for either a scatterplot or histogram along the main diagonal (parameter: diagonal).
Things I am working on implementing: