{"action":"created","issue":{"url":"https://api.github.com/repos/bigdatagenomics/adam/issues/553","labels_url":"https://api.github.com/repos/bigdatagenomics/adam/issues/553/labels{/name}","comments_url":"https://api.github.com/repos/bigdatagenomics/adam/issues/553/comments","events_url":"https://api.github.com/repos/bigdatagenomics/adam/issues/553/events","html_url":"https://github.com/bigdatagenomics/adam/issues/553","id":55235572,"number":553,"title":"Simplify `ADAMContext`","user":{"login":"laserson","id":118141,"avatar_url":"https://avatars.githubusercontent.com/u/118141?v=3","gravatar_id":"","url":"https://api.github.com/users/laserson","html_url":"https://github.com/laserson","followers_url":"https://api.github.com/users/laserson/followers","following_url":"https://api.github.com/users/laserson/following{/other_user}","gists_url":"https://api.github.com/users/laserson/gists{/gist_id}","starred_url":"https://api.github.com/users/laserson/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/laserson/subscriptions","organizations_url":"https://api.github.com/users/laserson/orgs","repos_url":"https://api.github.com/users/laserson/repos","events_url":"https://api.github.com/users/laserson/events{/privacy}","received_events_url":"https://api.github.com/users/laserson/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2015-01-23T01:55:18Z","updated_at":"2015-01-24T00:05:22Z","closed_at":null,"body":"I am finding `ADAMContext` to be more complicated than I'd like, especially when I am thinking about the Python API and the `JavaADAMContext`.  Here are a few questions:\r\n\r\n* What is the purpose of `adamLoad`?  We only have a relatively small number of types.  Does it make sense to break `adamLoad` out to the non-generic cases we are interested in?  We already have a `loadAlignments` function.\r\n\r\n* What is the purpose of `adamDictionaryLoad`?  It looks like it's basically an agg on an `RDD[AlignmentRecord]`.  Why don't we have the user load the RDD themselves, and then have a function `RDD[AlignmentRecord] => SequenceDictionary`?  If the use wants to do it on a `.bam` file, she can use `adamBamDictionaryLoad`.\r\n\r\n* What is the purpose of `maybeLoadBam`?  Make the user check that the file has the proper extension.  How is it useful to return an `Option`?\r\n\r\n* What's the purpose of `FeaturesContext`?  It just adds some loading functions.  Could be moved to `ADAMContext`.  Same with `VariationContext`.\r\n\r\n* What's the purpose of `GeneContext`?  Seems simpler to implement it by loading GTF `Feature` objects and then transforming the `RDD[Feature]` to `RDD[Gene]`.  Actually, this is what it does, but the implicit pimp-my-lib architecture just seems more confusing.\r\n\r\n* `VariantContext` is named strangely as well.\r\n\r\n* It's be great to consolidate some of the `*RDDFunctions` whenever possible, though not as obvious to me yet exactly what/how.\r\n\r\nA lot of this automagically calling functions by looking at extensions seems like over-complication, and not really that useful.  How often will it happen that the user doesn't know whether they're loading data from a `.bam` file or an `.adam` file?\r\n\r\nWhy are there so many `*Context` objects?\r\n\r\nThoughts on cleanup/simplification?"},"comment":{"url":"https://api.github.com/repos/bigdatagenomics/adam/issues/comments/71288006","html_url":"https://github.com/bigdatagenomics/adam/issues/553#issuecomment-71288006","issue_url":"https://api.github.com/repos/bigdatagenomics/adam/issues/553","id":71288006,"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":"2015-01-24T00:05:22Z","updated_at":"2015-01-24T00:05:22Z","body":"This is partially my fault. I split ADAMContext up in #356, to clean up several other issues. That being said, I do think that we should eliminate as much from the `*Context`s as is possible.\r\n\r\n> * What is the purpose of adamLoad? We only have a relatively small number of types. Does it make sense to break adamLoad out to the non-generic cases we are interested in? We already have a loadAlignments function.\r\n\r\nI'm a big fan of this; @erictu's #542 extended @ryan-williams #468 to genotypes, and I think that's the correct way to go for the other data types as well.\r\n\r\n> * What is the purpose of adamDictionaryLoad? It looks like it's basically an agg on an RDD[AlignmentRecord]. Why don't we have the user load the RDD themselves, and then have a function RDD[AlignmentRecord] => SequenceDictionary? If the use wants to do it on a .bam file, she can use adamBamDictionaryLoad.\r\n\r\nWe do have a function that goes `RDD[AlignmentRecord] => SequenceDictionary`; I agree RE: nixing that.\r\n\r\n> * What is the purpose of maybeLoadBam? Make the user check that the file has the proper extension. How is it useful to return an Option?\r\n\r\nJust merged #554, so no longer an issue! Thanks @ryan-williams! I'll fix `maybeLoadVcf` in a second.\r\n\r\n> * What's the purpose of FeaturesContext? It just adds some loading functions. Could be moved to ADAMContext. Same with VariationContext.\r\n\r\n+1; can we roll all the ADAMContext's together?\r\n\r\n> * What's the purpose of GeneContext? Seems simpler to implement it by loading GTF Feature objects and then transforming the RDD[Feature] to RDD[Gene]. Actually, this is what it does, but the implicit pimp-my-lib architecture just seems more confusing.\r\n\r\nShouldn't that be in `FeatureContext` anyways (if one exists)? I don't have a strong feeling here.\r\n\r\n> * VariantContext is named strangely as well.\r\n\r\nAh, yeah, that's a legacy thing. Originally, the name mirrored the Picard `VariantContext`. I'd be down with changing it.\r\n\r\n> * It's be great to consolidate some of the *RDDFunctions whenever possible, though not as obvious to me yet exactly what/how.\r\n\r\nSo, you can't really consolidate them down into a single class, because they all take different constructors and have different types. We used to have them in a single file; I didn't love that myself, TBH."}}