From 1ec8fc3e8bc982115e5ba1b16a101d239a2df591 Mon Sep 17 00:00:00 2001 From: gmungoc Date: Tue, 23 May 2017 12:37:10 +0100 Subject: [PATCH] JAL-2490 tidy code and tests for feature export in Jalview format --- src/jalview/appletgui/AlignFrame.java | 4 +- .../datamodel/features/SequenceFeatures.java | 2 + src/jalview/gui/AnnotationExporter.java | 4 +- src/jalview/io/FeaturesFile.java | 76 ++++++-------------- .../datamodel/features/SequenceFeaturesTest.java | 10 +-- test/jalview/io/FeaturesFileTest.java | 17 ++--- 6 files changed, 38 insertions(+), 75 deletions(-) diff --git a/src/jalview/appletgui/AlignFrame.java b/src/jalview/appletgui/AlignFrame.java index f914108..b8700e1 100644 --- a/src/jalview/appletgui/AlignFrame.java +++ b/src/jalview/appletgui/AlignFrame.java @@ -1447,12 +1447,12 @@ public class AlignFrame extends EmbmenuFrame implements ActionListener, if (format.equalsIgnoreCase("Jalview")) { features = formatter.printJalviewFormat(viewport.getAlignment() - .getSequencesArray(), getDisplayedFeatureCols()); + .getSequencesArray(), getDisplayedFeatureCols(), true); } else { features = formatter.printGffFormat(viewport.getAlignment() - .getSequencesArray(), getDisplayedFeatureCols()); + .getSequencesArray(), getDisplayedFeatureCols(), true); } if (displayTextbox) diff --git a/src/jalview/datamodel/features/SequenceFeatures.java b/src/jalview/datamodel/features/SequenceFeatures.java index 34470cc..ff570b1 100644 --- a/src/jalview/datamodel/features/SequenceFeatures.java +++ b/src/jalview/datamodel/features/SequenceFeatures.java @@ -220,9 +220,11 @@ public class SequenceFeatures implements SequenceFeaturesI /* * else make a copy of the list, and remove any null value just in case, * as it would cause errors looking up the features Map + * sort in alphabetical order for consistent output behaviour */ List types = new ArrayList(Arrays.asList(type)); types.remove(null); + Collections.sort(types); return types; } diff --git a/src/jalview/gui/AnnotationExporter.java b/src/jalview/gui/AnnotationExporter.java index cffc52f..a48c030 100644 --- a/src/jalview/gui/AnnotationExporter.java +++ b/src/jalview/gui/AnnotationExporter.java @@ -163,13 +163,13 @@ public class AnnotationExporter extends JPanel boolean includeNonPositional = ap.av.isShowNPFeats(); if (GFFFormat.isSelected()) { - text = formatter.printGffFormat(sequences, featureColours, true, + text = formatter.printGffFormat(sequences, featureColours, includeNonPositional); } else { text = formatter.printJalviewFormat(sequences, featureColours, - true, includeNonPositional); + includeNonPositional); } } else diff --git a/src/jalview/io/FeaturesFile.java b/src/jalview/io/FeaturesFile.java index 028b14f..869b18b 100755 --- a/src/jalview/io/FeaturesFile.java +++ b/src/jalview/io/FeaturesFile.java @@ -497,43 +497,24 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI { sf.addLink(link); } - } /** - * generate a features file for seqs includes non-pos features by default. - * - * @param sequences - * source of sequence features - * @param visible - * hash of feature types and colours - * @return features file contents - */ - public String printJalviewFormat(SequenceI[] sequences, - Map visible) - { - return printJalviewFormat(sequences, visible, true, true); - } - - /** - * generate a features file for seqs with colours from visible (if any) + * Returns contents of a Jalview format features file * * @param sequences * source of features * @param visible - * hash of Colours for each feature type - * @param visOnly - * when true only feature types in 'visible' will be output - * @param nonpos - * indicates if non-positional features should be output (regardless - * of group or type) - * @return features file contents + * map of colour for each visible feature type + * @param includeNonPositional + * if true, include non-positional features (regardless of group or + * type) + * @return */ public String printJalviewFormat(SequenceI[] sequences, - Map visible, boolean visOnly, - boolean nonpos) + Map visible, boolean includeNonPositional) { - if (visOnly && !nonpos && (visible == null || visible.isEmpty())) + if (!includeNonPositional && (visible == null || visible.isEmpty())) { // no point continuing. return "No Features Visible"; @@ -544,7 +525,7 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI */ // TODO: decide if feature links should also be written here ? StringBuilder out = new StringBuilder(256); - if (visible != null && visOnly) // todo why visOnly test? + if (visible != null) { for (Entry featureColour : visible.entrySet()) { @@ -556,14 +537,14 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI // Work out which groups are both present and visible Set groups = new HashSet(); - String[] types = visible == null ? null : visible.keySet().toArray( - new String[visible.keySet().size()]); + String[] types = visible == null ? new String[0] : visible.keySet() + .toArray(new String[visible.keySet().size()]); for (int i = 0; i < sequences.length; i++) { groups.addAll(sequences[i].getFeatures() .getFeatureGroups(true, types)); - if (nonpos) + if (includeNonPositional) { groups.addAll(sequences[i].getFeatures().getFeatureGroups(false, types)); @@ -594,13 +575,16 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI for (int i = 0; i < sequences.length; i++) { List features = new ArrayList(); - if (nonpos) + if (includeNonPositional) { features.addAll(sequences[i].getFeatures().getFeaturesForGroup( false, group, types)); } - features.addAll(sequences[i].getFeatures().getFeaturesForGroup( - true, group, types)); + if (types.length > 0) + { + features.addAll(sequences[i].getFeatures().getFeaturesForGroup( + true, group, types)); + } for (SequenceFeature sequenceFeature : features) { @@ -722,34 +706,17 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI } /** - * Returns features output in GFF2 format, including hidden and non-positional - * features - * - * @param sequences - * the sequences whose features are to be output - * @param visible - * a map whose keys are the type names of visible features - * @return - */ - public String printGffFormat(SequenceI[] sequences, - Map visible) - { - return printGffFormat(sequences, visible, true, true); - } - - /** * Returns features output in GFF2 format * * @param sequences * the sequences whose features are to be output * @param visible * a map whose keys are the type names of visible features - * @param outputVisibleOnly * @param includeNonPositionalFeatures * @return */ public String printGffFormat(SequenceI[] sequences, - Map visible, boolean outputVisibleOnly, + Map visible, boolean includeNonPositionalFeatures) { StringBuilder out = new StringBuilder(256); @@ -772,10 +739,7 @@ public class FeaturesFile extends AlignFile implements FeaturesSourceI */ continue; } - // TODO why the test !isnonpos here? - // what about not visible non-positional features? - if (!isnonpos && outputVisibleOnly - && !visible.containsKey(sf.type)) + if (!isnonpos && !visible.containsKey(sf.type)) { /* * ignore not visible features if not wanted diff --git a/test/jalview/datamodel/features/SequenceFeaturesTest.java b/test/jalview/datamodel/features/SequenceFeaturesTest.java index d3927fe..eaa5276 100644 --- a/test/jalview/datamodel/features/SequenceFeaturesTest.java +++ b/test/jalview/datamodel/features/SequenceFeaturesTest.java @@ -896,14 +896,14 @@ public class SequenceFeaturesTest assertFalse(iterator.hasNext()); /* - * two types specified + * two types specified - get sorted alphabetically */ types = sf.varargToTypes("Metal", "Helix"); iterator = types.iterator(); assertTrue(iterator.hasNext()); - assertEquals(iterator.next(), "Metal"); - assertTrue(iterator.hasNext()); assertEquals(iterator.next(), "Helix"); + assertTrue(iterator.hasNext()); + assertEquals(iterator.next(), "Metal"); assertFalse(iterator.hasNext()); /* @@ -912,9 +912,9 @@ public class SequenceFeaturesTest types = sf.varargToTypes("Metal", null, "Helix"); iterator = types.iterator(); assertTrue(iterator.hasNext()); - assertEquals(iterator.next(), "Metal"); - assertTrue(iterator.hasNext()); assertEquals(iterator.next(), "Helix"); + assertTrue(iterator.hasNext()); + assertEquals(iterator.next(), "Metal"); assertFalse(iterator.hasNext()); } diff --git a/test/jalview/io/FeaturesFileTest.java b/test/jalview/io/FeaturesFileTest.java index be5e68d..70a5f16 100644 --- a/test/jalview/io/FeaturesFileTest.java +++ b/test/jalview/io/FeaturesFileTest.java @@ -421,33 +421,31 @@ public class FeaturesFileTest FeatureRenderer fr = af.alignPanel.getFeatureRenderer(); Map visible = fr.getDisplayedFeatureCols(); String exported = featuresFile.printJalviewFormat( - al.getSequencesArray(), visible, true, false); + al.getSequencesArray(), visible, false); String expected = "No Features Visible"; assertEquals(expected, exported); /* * include non-positional features */ - af.getViewport().setShowNPFeats(true); exported = featuresFile.printJalviewFormat(al.getSequencesArray(), - visible, true, true); + visible, true); expected = "\nSTARTGROUP\tuniprot\nCath\tFER_CAPAA\t-1\t0\t0\tDomain\t0.0\nENDGROUP\tuniprot\n"; assertEquals(expected, exported); /* * set METAL (in uniprot group) and GAMMA-TURN visible, but not Pfam */ - af.getViewport().setShowNPFeats(false); fr.setVisible("METAL"); fr.setVisible("GAMMA-TURN"); visible = fr.getDisplayedFeatureCols(); exported = featuresFile.printJalviewFormat(al.getSequencesArray(), - visible, true, false); + visible, false); expected = "METAL\tcc9900\n" + "GAMMA-TURN\tff0000|00ffff|20.0|95.0|below|66.0\n" + "\nSTARTGROUP\tuniprot\n" - + "Iron\tFER_CAPAA\t-1\t39\t39\tMETAL\t0.0\n" + "Turn\tFER_CAPAA\t-1\t36\t38\tGAMMA-TURN\t0.0\n" + + "Iron\tFER_CAPAA\t-1\t39\t39\tMETAL\t0.0\n" + "ENDGROUP\tuniprot\n"; assertEquals(expected, exported); @@ -457,17 +455,16 @@ public class FeaturesFileTest fr.setVisible("Pfam"); visible = fr.getDisplayedFeatureCols(); exported = featuresFile.printJalviewFormat(al.getSequencesArray(), - visible, true, false); + visible, false); /* - * note the order of feature types is uncontrolled - derives from - * FeaturesDisplayed.featuresDisplayed which is a HashSet + * features are output within group, ordered by sequence and by type */ expected = "METAL\tcc9900\n" + "Pfam\tff0000\n" + "GAMMA-TURN\tff0000|00ffff|20.0|95.0|below|66.0\n" + "\nSTARTGROUP\tuniprot\n" - + "Iron\tFER_CAPAA\t-1\t39\t39\tMETAL\t0.0\n" + "Turn\tFER_CAPAA\t-1\t36\t38\tGAMMA-TURN\t0.0\n" + + "Iron\tFER_CAPAA\t-1\t39\t39\tMETAL\t0.0\n" + "Pfam domainPfam_3_4\tFER_CAPAA\t-1\t20\t20\tPfam\t0.0\n" + "ENDGROUP\tuniprot\n"; assertEquals(expected, exported); -- 1.7.10.2