{"action":"created","issue":{"url":"https://api.github.com/repos/ga4gh/server/issues/1061","repository_url":"https://api.github.com/repos/ga4gh/server","labels_url":"https://api.github.com/repos/ga4gh/server/issues/1061/labels{/name}","comments_url":"https://api.github.com/repos/ga4gh/server/issues/1061/comments","events_url":"https://api.github.com/repos/ga4gh/server/issues/1061/events","html_url":"https://github.com/ga4gh/server/pull/1061","id":146056034,"number":1061,"title":"Updated implementation and tests","user":{"login":"bwalsh","id":47808,"avatar_url":"https://avatars.githubusercontent.com/u/47808?v=3","gravatar_id":"","url":"https://api.github.com/users/bwalsh","html_url":"https://github.com/bwalsh","followers_url":"https://api.github.com/users/bwalsh/followers","following_url":"https://api.github.com/users/bwalsh/following{/other_user}","gists_url":"https://api.github.com/users/bwalsh/gists{/gist_id}","starred_url":"https://api.github.com/users/bwalsh/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/bwalsh/subscriptions","organizations_url":"https://api.github.com/users/bwalsh/orgs","repos_url":"https://api.github.com/users/bwalsh/repos","events_url":"https://api.github.com/users/bwalsh/events{/privacy}","received_events_url":"https://api.github.com/users/bwalsh/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":1,"created_at":"2016-04-05T17:30:41Z","updated_at":"2016-04-15T18:15:32Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/ga4gh/server/pulls/1061","html_url":"https://github.com/ga4gh/server/pull/1061","diff_url":"https://github.com/ga4gh/server/pull/1061.diff","patch_url":"https://github.com/ga4gh/server/pull/1061.patch"},"body":"Resolves: #998 #940 #939 #938 #937 "},"comment":{"url":"https://api.github.com/repos/ga4gh/server/issues/comments/210572119","html_url":"https://github.com/ga4gh/server/pull/1061#issuecomment-210572119","issue_url":"https://api.github.com/repos/ga4gh/server/issues/1061","id":210572119,"user":{"login":"david4096","id":3795586,"avatar_url":"https://avatars.githubusercontent.com/u/3795586?v=3","gravatar_id":"","url":"https://api.github.com/users/david4096","html_url":"https://github.com/david4096","followers_url":"https://api.github.com/users/david4096/followers","following_url":"https://api.github.com/users/david4096/following{/other_user}","gists_url":"https://api.github.com/users/david4096/gists{/gist_id}","starred_url":"https://api.github.com/users/david4096/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/david4096/subscriptions","organizations_url":"https://api.github.com/users/david4096/orgs","repos_url":"https://api.github.com/users/david4096/repos","events_url":"https://api.github.com/users/david4096/events{/privacy}","received_events_url":"https://api.github.com/users/david4096/received_events","type":"User","site_admin":false},"created_at":"2016-04-15T18:15:32Z","updated_at":"2016-04-15T18:15:32Z","body":"Lots of code, awesome!\r\n\r\nIt would be great if we could split the update commits from the feature commits. There's enough code in here that github won't print out the diffs for the parts that matter most, so it's hard to do inline comments.\r\n\r\nCould the `_toGA4GH` function be decomposed at all?\r\n\r\nAlthough you know how excited I am about the swagger docs, it will complicate the merge to include them at this point. It's still not decided whether they'll be included as part of the package or from some separate location.\r\n\r\nIn `genotypePhenotypeGenerator` an exception is thrown for a bad request but it looks like the same situation is attempting to be handled on line 305 of `genotype_phenotype.py`, which throws an not implemented. Sorry if I'm mixing cases, just wondering if one is more correct than the other, or perhaps they are on different code paths.\r\n\r\nThe offsets are being calculated in the beginning of that generator but aren't used (probably to go with the TODO for using SPARQL for paging).\r\n\r\nIf the PhenotypeAssociationSet is particularly tailored to the cgd database, as suggested by the comment, does it make sense to name it as such? This would make the path to PASets backed by other data more clear.\r\n\r\nThis is great work! I see there are a couple of problems with the protocol_definitions file, you may want to regenerate this using `scripts/process_schemas`. Does your branch of schemas include VA code? That might be the source of the problem. It seems like it doesn't have the VA stuff fully merged in."}}