{"action":"created","issue":{"url":"https://api.github.com/repos/dnouri/nolearn/issues/123","labels_url":"https://api.github.com/repos/dnouri/nolearn/issues/123/labels{/name}","comments_url":"https://api.github.com/repos/dnouri/nolearn/issues/123/comments","events_url":"https://api.github.com/repos/dnouri/nolearn/issues/123/events","html_url":"https://github.com/dnouri/nolearn/pull/123","id":95141647,"number":123,"title":"Add ability to weight training samples and predefine training/validation splits","user":{"login":"ledbetdr","id":4002411,"avatar_url":"https://avatars.githubusercontent.com/u/4002411?v=3","gravatar_id":"","url":"https://api.github.com/users/ledbetdr","html_url":"https://github.com/ledbetdr","followers_url":"https://api.github.com/users/ledbetdr/followers","following_url":"https://api.github.com/users/ledbetdr/following{/other_user}","gists_url":"https://api.github.com/users/ledbetdr/gists{/gist_id}","starred_url":"https://api.github.com/users/ledbetdr/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/ledbetdr/subscriptions","organizations_url":"https://api.github.com/users/ledbetdr/orgs","repos_url":"https://api.github.com/users/ledbetdr/repos","events_url":"https://api.github.com/users/ledbetdr/events{/privacy}","received_events_url":"https://api.github.com/users/ledbetdr/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2015-07-15T09:11:56Z","updated_at":"2015-07-15T10:32:11Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/dnouri/nolearn/pulls/123","html_url":"https://github.com/dnouri/nolearn/pull/123","diff_url":"https://github.com/dnouri/nolearn/pull/123.diff","patch_url":"https://github.com/dnouri/nolearn/pull/123.patch"},"body":"Enables the ability to utilize sklearn's PredefinedSplit to allow specific training/validation splits by hijacking the eval_size parameter. This could also be fairly easily split into its own parameter.\r\n\r\nMore involved are the changes to allow weights to be specified during the fit routine to leverage the refactored aggregate functionality in Lasagne. Removes use of the now deprecated Objective object."},"comment":{"url":"https://api.github.com/repos/dnouri/nolearn/issues/comments/121569454","html_url":"https://github.com/dnouri/nolearn/pull/123#issuecomment-121569454","issue_url":"https://api.github.com/repos/dnouri/nolearn/issues/123","id":121569454,"user":{"login":"dnouri","id":56370,"avatar_url":"https://avatars.githubusercontent.com/u/56370?v=3","gravatar_id":"","url":"https://api.github.com/users/dnouri","html_url":"https://github.com/dnouri","followers_url":"https://api.github.com/users/dnouri/followers","following_url":"https://api.github.com/users/dnouri/following{/other_user}","gists_url":"https://api.github.com/users/dnouri/gists{/gist_id}","starred_url":"https://api.github.com/users/dnouri/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/dnouri/subscriptions","organizations_url":"https://api.github.com/users/dnouri/orgs","repos_url":"https://api.github.com/users/dnouri/repos","events_url":"https://api.github.com/users/dnouri/events{/privacy}","received_events_url":"https://api.github.com/users/dnouri/received_events","type":"User","site_admin":false},"created_at":"2015-07-15T10:32:11Z","updated_at":"2015-07-15T10:32:11Z","body":"Thanks for your work.  I would have an easier time to digest this pull request if it were split into two parts: one for the aggregate/weights/objective, another for PredefinedSplit.\r\n\r\nUntil now, we've recommended people to subclass and overwrite `train_test_split` if they need another train/validation split.  It was suggested elsewhere that a function with the same signature as `train_test_split` should be passed as a parameter to `NeuralNet`, instead of forcing users to subclass.  If you think `PredefinedSplit` would be more convenient than that, then please explain.\r\n\r\nAs for the removal of Objective: it's long overdue.  But I think we want a convenient replacement that allows losses to be calculated using outputs of potentially multiple layers.  See here: https://github.com/dnouri/nolearn/issues/105#issuecomment-106774786\r\n\r\nAs you saw, there's also test failures caused by your changes.  Although these have lower priority than figuring out the questions from above, to get your pull request merged, you'll need to make sure that all tests run, and that you add tests for any additional functionality.  (Click on Details in the line that says \"Some checks have failed\" in red.)"}}