Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
@ldgauthier
ldgauthier committed Mar 1, 2019
1 parent 9e31649 commit b1ad96b
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 107 deletions.
74 changes: 20 additions & 54 deletions src/main/java/org/broadinstitute/hellbender/tools/walkers/variantutils/VariantsToTable.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,8 @@
)
@DocumentedFeature
public final class VariantsToTable extends VariantWalker {
private static final VariantsToTable instance = new VariantsToTable();
public final static String SPLIT_MULTI_ALLELIC_LONG_NAME = "split-multi-allelic";
public final static String SPLIT_MULTI_ALLELIC_SHORT_NAME = "SMA";

static final Logger logger = LogManager.getLogger(VariantsToTable.class);

Expand Down Expand Up @@ -147,7 +148,7 @@ public final class VariantsToTable extends VariantWalker {
@Argument(shortName="ASF", doc="The name of an allele-specific INFO field to be split if present", optional=true)
private List<String> asFieldsToTake = new ArrayList<>();

@Argument(shortName="ASGF", doc="The name of an allele-specific INFO field to be split if present", optional=true)
@Argument(shortName="ASGF", doc="The name of an allele-specific FORMAT field to be split if present", optional=true)
private List<String> asGenotypeFieldsToTake = new ArrayList<>();

/**
Expand All @@ -165,8 +166,8 @@ public final class VariantsToTable extends VariantWalker {
* (e.g. allele depth) separated by commas. This may cause difficulty when the table is loaded by an R script, for example.
* Use this flag to write multi-allelic records on separate lines of output. Fields that are not allele-specific will be duplicated.
*/
@Argument(fullName="split-multi-allelic",
shortName="SMA",
@Argument(fullName=SPLIT_MULTI_ALLELIC_LONG_NAME,
shortName=SPLIT_MULTI_ALLELIC_SHORT_NAME,
doc="Split multi-allelic records into multiple lines", optional=true)
protected boolean splitMultiAllelic = false;

Expand Down Expand Up @@ -228,34 +229,22 @@ public void onTraversalStart() {
}
}

if (asGenotypeFieldsToTake.isEmpty() && asFieldsToTake.isEmpty() && (!splitMultiAllelic)) {
logger.warn("Allele-specific fields will only be split if splitting multi-allelic variants is specified (`--split-multi-allelics` or `-SMA`");
if (asGenotypeFieldsToTake.isEmpty() && asFieldsToTake.isEmpty() && !splitMultiAllelic) {
logger.warn("Allele-specific fields will only be split if splitting multi-allelic variants is specified (`--" + SPLIT_MULTI_ALLELIC_LONG_NAME + "` or `-" + SPLIT_MULTI_ALLELIC_SHORT_NAME + "`");
}

// print out the header
if ( moltenizeOutput ) {
outputStream.println("RecordID\tSample\tVariable\tValue");
} else {
final String baseHeader = Utils.join("\t", fieldsToTake);
final String asBaseHeader = Utils.join("\t", asFieldsToTake);
final String genotypeHeader = createGenotypeHeader();
String line = "";
if (!baseHeader.isEmpty()) {
line += baseHeader;
}
if (!line.isEmpty() && !asBaseHeader.isEmpty()) {
line += "\t";
}
if (!asBaseHeader.isEmpty()) {
line += asBaseHeader;
}
if (!line.isEmpty() && !genotypeHeader.isEmpty()) {
line += "\t";
}
if (!genotypeHeader.isEmpty()) {
line += genotypeHeader;
}
outputStream.println(line);
final List<String> fields = new ArrayList<>();
fields.addAll(fieldsToTake);
fields.addAll(asFieldsToTake);
final String header = new StringBuilder(Utils.join("\t", fields))
.append("\t")
.append(createGenotypeHeader())
.toString();
outputStream.println(header);
}
}

Expand Down Expand Up @@ -286,21 +275,12 @@ private static boolean isWildCard(final String s) {

private String createGenotypeHeader() {
boolean firstEntry = true;
final List<String> allGenotypeFieldsToTake = new ArrayList<>(genotypeFieldsToTake);
allGenotypeFieldsToTake.addAll(asGenotypeFieldsToTake);

final StringBuilder sb = new StringBuilder();
for ( final String sample : samples ) {
for ( final String gf : genotypeFieldsToTake ) {
if ( firstEntry ) {
firstEntry = false;
} else {
sb.append("\t");
}
// spaces in sample names are legal but wreak havoc in R data frames
sb.append(sample.replace(" ","_"));
sb.append('.');
sb.append(gf);
}
for ( final String gf : asGenotypeFieldsToTake ) {
for ( final String gf : allGenotypeFieldsToTake ) {
if ( firstEntry ) {
firstEntry = false;
} else {
Expand Down Expand Up @@ -338,18 +318,8 @@ protected List<List<String>> extractFields(final VariantContext vc) {
final int numRecordsToProduce = splitMultiAllelic ? vc.getAlternateAlleles().size() : 1;
final List<List<String>> records = new ArrayList<>(numRecordsToProduce);

final int numFields;
final boolean hasGFields = genotypeFieldsToTake != null && !genotypeFieldsToTake.isEmpty();
final boolean hasASGFields = asGenotypeFieldsToTake != null && !asGenotypeFieldsToTake.isEmpty();
final boolean addGenotypeFields = hasGFields || hasASGFields;
if ( addGenotypeFields ) {
numFields = fieldsToTake.size() + genotypeFieldsToTake.size() * samples.size();
} else {
numFields = fieldsToTake.size();
}

for ( int i = 0; i < numRecordsToProduce; i++ ) {
records.add(new ArrayList<>(numFields));
records.add(new ArrayList<>());
}

for ( final String field : fieldsToTake ) {
Expand Down Expand Up @@ -388,7 +358,7 @@ protected List<List<String>> extractFields(final VariantContext vc) {
}
}

if ( addGenotypeFields ) {
if ( !genotypeFieldsToTake.isEmpty() || !asGenotypeFieldsToTake.isEmpty() ) {
addGenotypeFieldsToRecords(vc, records, errorIfMissingData);
}

Expand Down Expand Up @@ -583,8 +553,4 @@ private static Object splitAltAlleles(final VariantContext vc) {

return vc.getAlternateAlleles();
}

protected static VariantsToTable getInstance() {
return instance;
}
}
53 changes: 0 additions & 53 deletions ...ava/org/broadinstitute/hellbender/tools/walkers/variantutils/VariantsToTableUnitTest.java
View file

This file was deleted.

0 comments on commit b1ad96b

Please sign in to comment.