search

probcomp / iventure

An interactive, browser-based probabilistic programming environment.
Apache License 2.0
14 stars 2 forks source link

added seaborn pairplot command: %bql .pairplot <sqlexp> #75

Closed vkmvkmvkmvkm closed 4 years ago

vkmvkmvkmvkm commented 6 years ago

This commit adds a seaborn-based pairplot command to iVenture. It's been anecdotally tested, but no new tests for it have been added.

I have two questions:

  1. What else do I need to do before this is ready to merge? Write tests?

  2. What docs should I read (or have read) before putting this pull request together?

fsaad commented 6 years ago

@vkmvkmvkmvkm This is unlikely to work for data frames that have categorical/non-numeric variables.

A full implementation of pairplot which handles all three cases (numeric-numeric, numeric-categorical, and categorical-categorical) correctly, and is robust to messy data formats that arise in BayesDB tables, takes a few hundred lines of custom python (see e.g. Baxter's implementation in bdbcontrib, the javascript vizgpm code is similarly complex).

If pairplotting with nominal variables is not intended for now, we should rename this method to pairplot_numerical. If user provides a table with nominal columns, options are to either crash incomprehensibly or report (by checking the pd.dtypes) that nominals are not supported.

fsaad commented 6 years ago
  1. What else do I need to do before this is ready to merge? Write tests?

A minimal test in tests/test_plot_smoke.py would suffice. The iventure repo is not extensively tested since most features are interactive.

  1. What docs should I read (or have read) before putting this pull request together?

Contribution guidelines for PRs are in CONTRIBUTING.md