{"action":"created","issue":{"url":"https://api.github.com/repos/topepo/parsnip/issues/41","repository_url":"https://api.github.com/repos/topepo/parsnip","labels_url":"https://api.github.com/repos/topepo/parsnip/issues/41/labels{/name}","comments_url":"https://api.github.com/repos/topepo/parsnip/issues/41/comments","events_url":"https://api.github.com/repos/topepo/parsnip/issues/41/events","html_url":"https://github.com/topepo/parsnip/issues/41","id":348424695,"node_id":"MDU6SXNzdWUzNDg0MjQ2OTU=","number":41,"title":"Separate backend for tidy prediction","user":{"login":"alexpghayes","id":15078251,"node_id":"MDQ6VXNlcjE1MDc4MjUx","avatar_url":"https://avatars3.githubusercontent.com/u/15078251?v=4","gravatar_id":"","url":"https://api.github.com/users/alexpghayes","html_url":"https://github.com/alexpghayes","followers_url":"https://api.github.com/users/alexpghayes/followers","following_url":"https://api.github.com/users/alexpghayes/following{/other_user}","gists_url":"https://api.github.com/users/alexpghayes/gists{/gist_id}","starred_url":"https://api.github.com/users/alexpghayes/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexpghayes/subscriptions","organizations_url":"https://api.github.com/users/alexpghayes/orgs","repos_url":"https://api.github.com/users/alexpghayes/repos","events_url":"https://api.github.com/users/alexpghayes/events{/privacy}","received_events_url":"https://api.github.com/users/alexpghayes/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":19,"created_at":"2018-08-07T17:53:22Z","updated_at":"2018-08-09T17:01:42Z","closed_at":null,"author_association":"NONE","body":"I've been reworking the `augment()` methods and it's rapidly becoming clear that dealing with idiosyncratic `predict()` methods is going to slow down progress immensely.\r\n\r\nIn the end `broom` and `parsnip` are both going to want to wrap a bajillion predict methods, and we should report predictions in the same way for consistencies sake. I think we should move this functionality to a separate package. Potentially we could use the [`prediction`](https://github.com/leeper/prediction) package by Thomas Leeper, but we should decide on the behavior we want first.\r\n\r\nIf we define a new generic / series of generics, we can then test these behaviors in `modeltests` and allow other modelling package developers to guarantee that their `predict()` methods are sane and consistent.\r\n\r\n**What I want from a predict method**:\r\n\r\n- Returns a tidy tibble\r\n- Never drops missing data (i.e. matches the behavior of `predict.lm(..., na.action = na.pass)`)\r\n- Consistent naming of fitted values\r\n- Uncertainty in predictions\r\n\r\nI want all of these to be guaranteed, and for methods that cannot meet these guarantees, I want an informative error rather than a partially correct output.\r\n\r\n"},"comment":{"url":"https://api.github.com/repos/topepo/parsnip/issues/comments/411827551","html_url":"https://github.com/topepo/parsnip/issues/41#issuecomment-411827551","issue_url":"https://api.github.com/repos/topepo/parsnip/issues/41","id":411827551,"node_id":"MDEyOklzc3VlQ29tbWVudDQxMTgyNzU1MQ==","user":{"login":"alexpghayes","id":15078251,"node_id":"MDQ6VXNlcjE1MDc4MjUx","avatar_url":"https://avatars3.githubusercontent.com/u/15078251?v=4","gravatar_id":"","url":"https://api.github.com/users/alexpghayes","html_url":"https://github.com/alexpghayes","followers_url":"https://api.github.com/users/alexpghayes/followers","following_url":"https://api.github.com/users/alexpghayes/following{/other_user}","gists_url":"https://api.github.com/users/alexpghayes/gists{/gist_id}","starred_url":"https://api.github.com/users/alexpghayes/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/alexpghayes/subscriptions","organizations_url":"https://api.github.com/users/alexpghayes/orgs","repos_url":"https://api.github.com/users/alexpghayes/repos","events_url":"https://api.github.com/users/alexpghayes/events{/privacy}","received_events_url":"https://api.github.com/users/alexpghayes/received_events","type":"User","site_admin":false},"created_at":"2018-08-09T17:01:42Z","updated_at":"2018-08-09T17:01:42Z","author_association":"NONE","body":"Naming: I'm not a huge fan of `pred`. See also some [conventions](http://mjskay.github.io/tidybayes/reference/tidybayes-deprecated.html#deprecated-arguments-and-column-names) the Stan crew just went with. I think full singular nouns is the way to go.\r\n\r\n>  The return tibble can contain extra attributes for values relevant to the prediction (e.g. alpha for intervals) but care should be taken to make sure that these attributes are not destroyed when standard operations are applied to the tibble (e.g. arrange, filter, etc.).\r\n\r\nI think that ideally as much information as possible should be contained in the tibble itself. Users are much more familiar working with tibbles than attributes. For example, for intervals, I'd strongly prefer a column `width` to an attribute. This would make working with multiple intervals at once nicer as well (for plotting, say).\r\n\r\n>    For class probability predictions, the columns should be named the same as the factor levels (with back-ticks when they are not valid column names).\r\n\r\nNo indication that the columns are probabilities in the name?\r\n\r\n> If interval estimates are produced (e.g. prediction/confidence/credible), the column names should be lower and upper. If a standard error is produced, it should be named std_error.\r\n\r\nYes.\r\n\r\n>  If a posterior distribution is returned for each sample, each element of the list column can be a tibble with as many rows as samples in the distribution.\r\n\r\nNot sold on this. For a high level interface, we should not force users to interact with posterior samples. We should provide a default summary of those samples and only optionally return them.\r\n\r\n> When a predict method produces values over multiple tuning parameter values (e.g. glmnet), the list column elements have rows for every tuning parameter value combination that is being changed (e.g. lambda in glmnet).\r\n\r\nI think we need to think very carefully if we want to do this. My take is that so far this entire thread has been defining a `predict()` method for a single fitted model object. When you have multiple hyperparameter combinations, you are fundamentally working with multiple fitted model objects. I'm not convinced it even makes sense to define `predict()` in this case. If we are going to define fit at all, I think one sensibly option is:\r\n\r\n```r\r\npredict.many_fits <- function(...) {\r\n   best_fit <- extract_best_fit(many_fits)\r\n   predict(best_fit)\r\n}\r\n```\r\n\r\nto get predictions for different hyperparameter values I'd much prefer a workflow like:\r\n\r\n```r\r\nmap(many_fits, predict) # many_fits basically a list of fitted objects\r\n```\r\n\r\nBut I think we should be really careful here.\r\n\r\n\r\n> If newdata is not supplied, an error should be thrown.\r\n\r\nStrongly second this, and came here to recommend it. This is an issue because models often drop rows with missing data silently, which leads to all sorts of nasty surprises and after the fact attempts to determine precisely which rows got dropped."}}
{"actor":{"display_login":"alexpghayes"}}