New function to sample from frames (Issue #2419 )
State: closed

closes #2419 
Creates a function `.sample()` in generic.py to sample from pandas objects. Returns either a fixed number of rows or a share. Also includes a number of tests in `test_sample()` -- I am open to added suggestions. 

Input from users with panel experience appreciated! 


Labels:
name: Enhancement

Comments:
Please excuse the many (meaningless) commits -- was learning to use git from commandline. 

This will also need documentation updates:
- An addition to "what's new" for v0.16.1 (not sure if this file has been created yet; best to wait until 0.16 is released)
- New documentation in an appropriate section of the docs
- Addition to the API docs (see `api.rst`).

Thanks @shoyer. Adding changes and and testing.

Doc additions soon -- wanted to pin down the function first, but may need to do some "real job" things before finishing. 

How about adding some tests to verify that the resulting frames/series have the right number of rows? These would also function as sanity checks to verify that sample is doing the right thing when you provide `frac` or `n` as arguments.

I have a few style/doc nits but this looks pretty good to me, pending documentation updates.

Great, thanks @shoyer. Once the 16.1 `what's new` is up I'll write up docs. 

whats new is now available for 0.16.1 https://github.com/pydata/pandas/commit/5ebf521eac528f5a4b772e4677f70b689b8b1826

Hi All: I added a note to whatsnew, and added DataFrame.sample and Series.sample everywhere that seemed relevant in the api.rst docs. 

Is there anything more I should be adding / can anyone point me to more detailed docs on contributing to documentation besides the readme.rst in pandas/docs?

@nickeubank It's really up to you where this belongs in the documentation! I might add a new section "Random sampling" to the "Indexing and selecting" portion of the documentation:
http://pandas.pydata.org/pandas-docs/version/0.16.0/indexing.html

You can find the source for this section in `doc/source/indexing.rst`

@shoyer -- oh! I thought that only applied to defining functions, I didn't realize that the no spaces applied in function calls. adding now. 

@nickeubank sorry for the delay looking at this again -- github doesn't send any notifications when a PR is updated, only when new comments appears.

This looks very good to me -- pretty much good to go, once you fix a few styling issues.

@TomAugspurger @jorisvandenbossche @cpcloud @jreback Any additional comments?

pls rebase and squash

This is becoming a really nice improvement!

Two questions:
- Now, `axis=0` is passed to the eventual `self.take(..)` call. Should this be configurable? (by having an `axis` argument in `sample`) E.g. for panels, I don't use them, but for those who do, maybe you want to be able to specify on which axis to sample?
- The default of `n=5`. This was discussed before, and there already some objections then. But still, this seems a bit strange to me. E.g. also the defaults of returning 1 sample (`n=1`, this is what `numpy.choice` does), or returning all rows in random order (`n=len(self)`) both do seem as sensible defaults to me.

It is nice if `seed` can accept `np.random.RandomState` also (like scikit-learn's `random_state` does). And I prefer keyword `random_state` to `seed` if there is no specific reason.

Any other comments on the `axis`, default of `n` and `random_state` questions?

- `axis` should prob added (and default to 0).
- `seed` should accept a random_state object / int (is this passable to `np.random.random_state`?)

scikit learn has a really nice little utility function, `check_random_state`: https://github.com/scikit-learn/scikit-learn/blob/2b3fabc/sklearn/utils/validation.py#L498

If we want a comprehensive solution, we should just directly copy it into pandas and use it to process the `seed` argument (or, perhaps we should also call it `random_state` [like scikit-learn](http://scikit-learn.org/stable/modules/generated/sklearn.decomposition.RandomizedPCA.html#sklearn.decomposition.RandomizedPCA)). scikit-learn is BSD licensed, some we'll just need to add a copy of its license to the `LICENSES` directory.

Thanks all -- traveling for conferences this week and next but these all look like great ideas, and i'll integrate them when I'm back!

@nickeubank can you rebase and squash this.

Thanks all for feedback. Now rebased and squashed, with the following changes (and associated tests):
- added axis argument (default 0). 
- seed is now called random_state and accepts either integers or numpy.random.RandomState objects.
- weights will now re-normalize weights that do not sum to one, and will set values of np.nan or None to zero. 
- default is now to return 1 row (as per wisdom of the crowd). 
- documentation updated. 

@nickeubank looks like you haven't pushed those fixes yet?

Sorry, noticed typo in documentation -- recompiling, then will push. 

OK, now pushed. Sorry for delay. 

@nickeubank I am adding a couple more of stylistic comments. But, I think you are mixing tabs and spaces for indentation. Can you check your editor to only use spaces? At least in the docs page there are some tabs

Some other comments / questions:
- When passing a series as `weights`, should the values of the series be aligned on the index? (this is not the case at the moment)
- Should the default `n=1` case for DataFrames return a 1-row DataFrame, or that row as a Series (and for sampling a Series -> return 1-element Series or a scalar)? As a comparison `np.random.choice` reduces the dimensionality for `n=1`, but in the case of pandas you then loose the info of the index. So maybe keep it as is (DataFrame -> 1-row DataFrame)?
- I know nothing about Panels (never used it), but should the default axis be 0? (for DataFrame's the info axis is 1, so by default samples from axis 0. But for panels the info axis is 0). Are there similar functions to look at to do this consistently?
- The `weights` don't work for Panels at the moment (and I also don't see how it could work?). But if this is by design, it should get an appropriate error message, testing and docs (the docstring now says: "If called on a DataFrame or Panel, will also accept the name of a column as a string" but this does not work)

Thanks @jorvisvandenbossche! 
- "should one row of df be returned as df or series?" I really dislike the idea that different arguments (`n=1` versus `n=2`) return different types of objects, so I strongly prefer it returning a one row `DataFrame` if that's the item being sampled. 
- I think it makes sense to just allow for strings to be passed as weights for `DataFrames` when sampling from rows. I starting writing it in a more general form to allow for rows as weights, but it gets very hairy fast, especially because rows may be of mixed types. Since sampling from rows is likely the most common use case, and one can just explicitly pass a row as a weight if needed, I think this is sufficient. Changing documentation accordingly, and tests added for appropriate errors added for panels.  
- I also know nothing about `panels` -- I was assuming that axis 0 and 1 are same as `DataFrame` and `axis=2` is just time, but sounds like that's incorrect? Input by someone who uses panels would be appreciated. 
- the `weights = weights.values` line does negate much of the value of using Series, but I've found that `np.choice()` is not consistent in its ability to work with `Series` as weights. When I tried passing a Series as weights, I found that it worked on my computer, but 3 of the 5 Travis builds kept failing. The `weights = weights.values` is the only way I could fix the problem. 
  - should the values of series be aligned: given `np.choice` won't take a Series, how would I go about this? If you can suggest a way to do this, I'd be happy to do so. 

Small things fixed:
- tabs were causing issues with format. All spaces now. 
- PEP-8 in function signatures fixed. Sorry about that. 

@jreback could you shed some light on the panel issues?

@nickeubank 
- agree on the `n=1` and `n>1` giving the same output type
- OK for me on limiting specifying `weights` as a string for now to DataFrame columns
- on the `weights = weights.values` thing, it just seems to me that if you want to end up with an array, the initial `weights = pd.Series(weights, dtype = 'float64')` is not needed? **EDIT** ah, I see you use `fillna`, for which you need it as a series 

some minor code comments.

if an axis is not provided, it is usual to use `axis = self._stat_axis`, this will yield:
`Series->0, DataFrame->0, Panel->1`. This type of code is used all over `generic.py`

pls squash.
otherwise lgtm. once you have made changes, ping when green.

@jreback : I think we're good to go. 

Added axis = self._stat_axis and associated tests; added is_integer() to _random_state(); thinned comments; fixed format in "what's new"; updated docs on only supporting column names for dataframes. 

@jreback @shoyer  @jorisvandenbossche @TomAugspurger @sinhrks : thank you all for your input on this -- it's been a great learning experience, and hopefully a useful new feature!