{"action":"created","issue":{"url":"https://api.github.com/repos/bigdatagenomics/adam/issues/1185","repository_url":"https://api.github.com/repos/bigdatagenomics/adam","labels_url":"https://api.github.com/repos/bigdatagenomics/adam/issues/1185/labels{/name}","comments_url":"https://api.github.com/repos/bigdatagenomics/adam/issues/1185/comments","events_url":"https://api.github.com/repos/bigdatagenomics/adam/issues/1185/events","html_url":"https://github.com/bigdatagenomics/adam/pull/1185","id":179685846,"number":1185,"title":"Fastq record converter","user":{"login":"zyxue","id":527862,"avatar_url":"https://avatars.githubusercontent.com/u/527862?v=3","gravatar_id":"","url":"https://api.github.com/users/zyxue","html_url":"https://github.com/zyxue","followers_url":"https://api.github.com/users/zyxue/followers","following_url":"https://api.github.com/users/zyxue/following{/other_user}","gists_url":"https://api.github.com/users/zyxue/gists{/gist_id}","starred_url":"https://api.github.com/users/zyxue/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/zyxue/subscriptions","organizations_url":"https://api.github.com/users/zyxue/orgs","repos_url":"https://api.github.com/users/zyxue/repos","events_url":"https://api.github.com/users/zyxue/events{/privacy}","received_events_url":"https://api.github.com/users/zyxue/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":null,"comments":25,"created_at":"2016-09-28T05:51:09Z","updated_at":"2016-09-29T21:46:24Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/bigdatagenomics/adam/pulls/1185","html_url":"https://github.com/bigdatagenomics/adam/pull/1185","diff_url":"https://github.com/bigdatagenomics/adam/pull/1185.diff","patch_url":"https://github.com/bigdatagenomics/adam/pull/1185.patch"},"body":"There are still problems:\r\n\r\n* The description for `convertRead` does don't match with what it does (e.g. the return type). The method name is kind of vague, not sure what behavior is actually intended. The refactoring tries not to change its behavior.\r\n* I am also a bit confused for what default values should be used when creating a AlignmentRecord instance based on a Fastq entry. The original code set many fields to `null`. I thought leaving them as default would make more sense. e.g. `ReadNegativeStrand` is set to `null` while the [default](https://github.com/bigdatagenomics/bdg-formats/blob/master/src/main/resources/avro/bdg.avdl) is `false`. `ProperPair` is set to `true`, which I don't think make sense since you can't really tell just based on Fastq, the default is also `fasle`. It would be helpful if someone can clarify what's the most sensible values for the following fields for both paired-end and single-end fastq entries.\r\n```\r\n.setReadPaired(readPaired)\r\n.setReadInFragment(readInFragment)\r\n.setReadNegativeStrand(null)\r\n.setMateNegativeStrand(null)\r\n.setPrimaryAlignment(null)\r\n.setSecondaryAlignment(null)\r\n.setSupplementaryAlignment(null)\r\n```\r\n\r\n* Also, what to do when the read length and that of qualities don't match? There is a stringency parameter involved in `convertRead`. If it's STRICT, then qualities must exist (CANNOT be `*`) and also match the length of reads. When it's NOT STRICT, qualities will be padded with `B` if not exist or shorter than read length. If it's longer than read length, `NotImplementedError` will be thrown. Such behavior seems quite arbitrary and doesn't make much sense to me, and it doesn't apply to `convertPair` and `convertFragment`, in which the read length and qualities must match without consideration for stringency."},"comment":{"url":"https://api.github.com/repos/bigdatagenomics/adam/issues/comments/250600626","html_url":"https://github.com/bigdatagenomics/adam/pull/1185#issuecomment-250600626","issue_url":"https://api.github.com/repos/bigdatagenomics/adam/issues/1185","id":250600626,"user":{"login":"fnothaft","id":3752466,"avatar_url":"https://avatars.githubusercontent.com/u/3752466?v=3","gravatar_id":"","url":"https://api.github.com/users/fnothaft","html_url":"https://github.com/fnothaft","followers_url":"https://api.github.com/users/fnothaft/followers","following_url":"https://api.github.com/users/fnothaft/following{/other_user}","gists_url":"https://api.github.com/users/fnothaft/gists{/gist_id}","starred_url":"https://api.github.com/users/fnothaft/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/fnothaft/subscriptions","organizations_url":"https://api.github.com/users/fnothaft/orgs","repos_url":"https://api.github.com/users/fnothaft/repos","events_url":"https://api.github.com/users/fnothaft/events{/privacy}","received_events_url":"https://api.github.com/users/fnothaft/received_events","type":"User","site_admin":false},"created_at":"2016-09-29T21:46:23Z","updated_at":"2016-09-29T21:46:23Z","body":"I just made a review pass, @zyxue. I've highlighted a few small syntax nits, but overall, this looks like a very solid PR; thank you for the great contribution!"}}
{"actor":{"display_login":"fnothaft"}}