hammerlab / guacamole

Spark-based variant calling, with experimental support for multi-sample somatic calling (including RNA) and local assembly
Apache License 2.0
83 stars 21 forks source link

remove method-access boilerplate #599

Closed ryan-williams closed 7 years ago

ryan-williams commented 7 years ago

In a lot of cases we import x.y.z.A and then reference A.{foo, bar, baz} throughout the file.

In cases where there is no ambiguity about where foo/bar/baz come from (which turns out to basically always be the case, just like with imports of classes where we almost always get away with importing all the way to the basename, avoiding including a disambiguating package-classifier or renamed import), it's cleaner to import x.y.z.A.{foo, bar, baz} and then reference them directly.

I've done this with several classes+methods here, broken out into individual commits.

coveralls commented 7 years ago

Coverage Status

Coverage decreased (-0.05%) to 78.657% when pulling 9eb74a3509b2aa51561265079a4494231caee555 on ryan-williams:bp into be6074747e1ae2f628d1d53be18e5eb091b611dd on hammerlab:master.

coveralls commented 7 years ago

Coverage Status

Changes Unknown when pulling 4fee1d158b94c3e6b2895223345287811de6c427 on ryan-williams:bp into \ on hammerlab:master**.

ryan-williams commented 7 years ago

Yea, I hear that, but the analogy to the way we import classes – all the way to the basename that is used, except where there is a collision between two imported things, in which case we qualify it in one of several available ways – is more compelling to me.

In most of these cases the containing-object-name was excruciatingly redundant (e.g. Bases.basestoString et al), hearkening to this pattern that I've worked against in the past as well (e.g. Likelihood.likelihoodOfGenotypes, PileupFlatMapUtils.pileupFlatMap*), and in all of them the methods being called are not ambiguous with anything else in scope, so I want to push hard in the other direction.

Thanks for the review!