Merge branch 'develop' into features/JAL-1793VCF; lambda Function for
authorgmungoc <g.m.carstairs@dundee.ac.uk>
Tue, 24 Oct 2017 12:45:15 +0000 (13:45 +0100)
committergmungoc <g.m.carstairs@dundee.ac.uk>
Tue, 24 Oct 2017 12:45:15 +0000 (13:45 +0100)
EnsemblLookup parsing

Conflicts:
src/jalview/ext/ensembl/EnsemblInfo.java
src/jalview/ext/ensembl/EnsemblLookup.java
src/jalview/ext/ensembl/EnsemblRestClient.java

1  2 
src/jalview/datamodel/Sequence.java
src/jalview/ext/ensembl/EnsemblGene.java
src/jalview/ext/ensembl/EnsemblLookup.java
src/jalview/ext/ensembl/EnsemblRestClient.java
src/jalview/gui/AlignFrame.java
src/jalview/gui/IdPanel.java
src/jalview/gui/PopupMenu.java
src/jalview/gui/SeqPanel.java
test/jalview/gui/SeqCanvasTest.java

@@@ -38,8 -38,6 +38,6 @@@ import java.util.List
  import java.util.ListIterator;
  import java.util.Vector;
  
- import com.stevesoft.pat.Regex;
  import fr.orsay.lri.varna.models.rna.RNA;
  
  /**
   */
  public class Sequence extends ASequence implements SequenceI
  {
-   private static final Regex limitrx = new Regex(
-           "[/][0-9]{1,}[-][0-9]{1,}$");
-   private static final Regex endrx = new Regex("[0-9]{1,}$");
    SequenceI datasetSequence;
  
    String name;
      checkValidRange();
    }
  
+   /**
+    * If 'name' ends in /i-j, where i >= j > 0 are integers, extracts i and j as
+    * start and end respectively and removes the suffix from the name
+    */
    void parseId()
    {
      if (name == null)
                "POSSIBLE IMPLEMENTATION ERROR: null sequence name passed to constructor.");
        name = "";
      }
-     // Does sequence have the /start-end signature?
-     if (limitrx.search(name))
+     int slashPos = name.lastIndexOf('/');
+     if (slashPos > -1 && slashPos < name.length() - 1)
      {
-       name = limitrx.left();
-       endrx.search(limitrx.stringMatched());
-       setStart(Integer.parseInt(limitrx.stringMatched().substring(1,
-               endrx.matchedFrom() - 1)));
-       setEnd(Integer.parseInt(endrx.stringMatched()));
+       String suffix = name.substring(slashPos + 1);
+       String[] range = suffix.split("-");
+       if (range.length == 2)
+       {
+         try
+         {
+           int from = Integer.valueOf(range[0]);
+           int to = Integer.valueOf(range[1]);
+           if (from > 0 && to >= from)
+           {
+             name = name.substring(0, slashPos);
+             setStart(from);
+             setEnd(to);
+             checkValidRange();
+           }
+         } catch (NumberFormatException e)
+         {
+           // leave name unchanged if suffix is invalid
+         }
+       }
      }
    }
  
+   /**
+    * Ensures that 'end' is not before the end of the sequence, that is,
+    * (end-start+1) is at least as long as the count of ungapped positions. Note
+    * that end is permitted to be beyond the end of the sequence data.
+    */
    void checkValidRange()
    {
      // Note: JAL-774 :
        int endRes = 0;
        for (int j = 0; j < sequence.length; j++)
        {
-         if (!jalview.util.Comparison.isGap(sequence[j]))
+         if (!Comparison.isGap(sequence[j]))
          {
            endRes++;
          }
    }
  
    /**
-    * DOCUMENT ME!
+    * Sets the sequence name. If the name ends in /start-end, then the start-end
+    * values are parsed out and set, and the suffix is removed from the name.
     * 
-    * @param name
-    *          DOCUMENT ME!
+    * @param theName
     */
    @Override
-   public void setName(String name)
+   public void setName(String theName)
    {
-     this.name = name;
+     this.name = theName;
      this.parseId();
    }
  
    }
  
    /**
 -   * DOCUMENT ME!
 +   * Sets the sequence description, and also parses out any special formats of
 +   * interest
     * 
     * @param desc
 -   *          DOCUMENT ME!
     */
    @Override
    public void setDescription(String desc)
      this.description = desc;
    }
  
 +  @Override
 +  public void setGeneLoci(String speciesId, String assemblyId,
 +          String chromosomeId, MapList map)
 +  {
 +    addDBRef(new DBRefEntry(speciesId, assemblyId, DBRefEntry.CHROMOSOME
 +            + ":" + chromosomeId, new Mapping(map)));
 +  }
 +
    /**
 -   * DOCUMENT ME!
 +   * Returns the gene loci mapping for the sequence (may be null)
     * 
 -   * @return DOCUMENT ME!
 +   * @return
 +   */
 +  @Override
 +  public GeneLociI getGeneLoci()
 +  {
 +    DBRefEntry[] refs = getDBRefs();
 +    if (refs != null)
 +    {
 +      for (final DBRefEntry ref : refs)
 +      {
 +        if (ref.isChromosome())
 +        {
 +          return new GeneLociI()
 +          {
 +            @Override
 +            public String getSpeciesId()
 +            {
 +              return ref.getSource();
 +            }
 +
 +            @Override
 +            public String getAssemblyId()
 +            {
 +              return ref.getVersion();
 +            }
 +
 +            @Override
 +            public String getChromosomeId()
 +            {
 +              // strip of "chromosome:" prefix to chrId
 +              return ref.getAccessionId().substring(
 +                      DBRefEntry.CHROMOSOME.length() + 1);
 +            }
 +
 +            @Override
 +            public MapList getMap()
 +            {
 +              return ref.getMap().getMap();
 +            }
 +          };
 +        }
 +      }
 +    }
 +    return null;
 +  }
 +
 +  /**
 +   * Answers the description
 +   * 
 +   * @return
     */
    @Override
    public String getDescription()
       * and we may have included adjacent or enclosing features;
       * remove any that are not enclosing, non-contact features
       */
-     if (endPos > this.end || Comparison.isGap(sequence[toColumn - 1]))
+     boolean endColumnIsGapped = toColumn > 0 && toColumn <= sequence.length
+             && Comparison.isGap(sequence[toColumn - 1]);
+     if (endPos > this.end || endColumnIsGapped)
      {
        ListIterator<SequenceFeature> it = result.listIterator();
        while (it.hasNext())
@@@ -23,8 -23,6 +23,8 @@@ package jalview.ext.ensembl
  import jalview.api.FeatureColourI;
  import jalview.api.FeatureSettingsModelI;
  import jalview.datamodel.AlignmentI;
 +import jalview.datamodel.DBRefEntry;
 +import jalview.datamodel.GeneLociI;
  import jalview.datamodel.Sequence;
  import jalview.datamodel.SequenceFeature;
  import jalview.datamodel.SequenceI;
@@@ -146,9 -144,6 +146,9 @@@ public class EnsemblGene extends Ensemb
        {
          continue;
        }
 +      
 +      parseChromosomeLocations(geneAlignment);
 +      
        if (geneAlignment.getHeight() == 1)
        {
          getTranscripts(geneAlignment, geneId);
    }
  
    /**
 +   * Parses and saves fields of an Ensembl-style description e.g.
 +   * chromosome:GRCh38:17:45051610:45109016:1
 +   * 
 +   * @param alignment
 +   */
 +  private void parseChromosomeLocations(AlignmentI alignment)
 +  {
 +    for (SequenceI seq : alignment.getSequences())
 +    {
 +      String description = seq.getDescription();
 +      if (description == null)
 +      {
 +        continue;
 +      }
 +      String[] tokens = description.split(":");
 +      if (tokens.length == 6 && tokens[0].startsWith(DBRefEntry.CHROMOSOME))
 +      {
 +        String ref = tokens[1];
 +        String chrom = tokens[2];
 +        try
 +        {
 +          int chStart = Integer.parseInt(tokens[3]);
 +          int chEnd = Integer.parseInt(tokens[4]);
 +          boolean forwardStrand = "1".equals(tokens[5]);
 +          String species = ""; // dunno yet!
 +          int[] from = new int[] { seq.getStart(), seq.getEnd() };
 +          int[] to = new int[] { forwardStrand ? chStart : chEnd,
 +              forwardStrand ? chEnd : chStart };
 +          MapList map = new MapList(from, to, 1, 1);
 +          seq.setGeneLoci(species, ref, chrom, map);
 +        } catch (NumberFormatException e)
 +        {
 +          System.err.println("Bad integers in description " + description);
 +        }
 +      }
 +    }
 +  }
 +
 +  /**
-    * Converts a query, which may contain one or more gene or transcript
-    * identifiers, into a non-redundant list of gene identifiers.
+    * Converts a query, which may contain one or more gene, transcript, or
+    * external (to Ensembl) identifiers, into a non-redundant list of gene
+    * identifiers.
     * 
     * @param accessions
     * @return
  
      for (String acc : accessions.split(getAccessionSeparator()))
      {
-       if (isGeneIdentifier(acc))
-       {
-         if (!geneIds.contains(acc))
-         {
-           geneIds.add(acc);
-         }
-       }
        /*
-        * if given a transcript id, look up its gene parent
+        * First try lookup as an Ensembl (gene or transcript) identifier
         */
-       else if (isTranscriptIdentifier(acc))
+       String geneId = new EnsemblLookup(getDomain()).getGeneId(acc);
+       if (geneId != null)
        {
-         String geneId = new EnsemblLookup(getDomain()).getParent(acc);
-         if (geneId != null && !geneIds.contains(geneId))
+         if (!geneIds.contains(geneId))
          {
            geneIds.add(geneId);
          }
        }
-       else if (isProteinIdentifier(acc))
-       {
-         String tscriptId = new EnsemblLookup(getDomain()).getParent(acc);
-         if (tscriptId != null)
-         {
-           String geneId = new EnsemblLookup(getDomain())
-                   .getParent(tscriptId);
-           if (geneId != null && !geneIds.contains(geneId))
-           {
-             geneIds.add(geneId);
-           }
-         }
-         // NOTE - acc is lost if it resembles an ENS.+ ID but isn't actually
-         // resolving to one... e.g. ENSMICP00000009241
-       }
-       /*
-        * if given a gene or other external name, lookup and fetch 
-        * the corresponding gene for all model organisms 
-        */
        else
        {
+         /*
+          * if given a gene or other external name, lookup and fetch 
+          * the corresponding gene for all model organisms 
+          */
          List<String> ids = new EnsemblSymbol(getDomain(), getDbSource(),
-                 getDbVersion()).getIds(acc);
-         for (String geneId : ids)
+                 getDbVersion()).getGeneIds(acc);
+         for (String id : ids)
          {
-           if (!geneIds.contains(geneId))
+           if (!geneIds.contains(id))
            {
-             geneIds.add(geneId);
+             geneIds.add(id);
            }
          }
        }
    }
  
    /**
-    * Attempts to get Ensembl stable identifiers for model organisms for a gene
-    * name by calling the xrefs symbol REST service to resolve the gene name.
-    * 
-    * @param query
-    * @return
-    */
-   protected String getGeneIdentifiersForName(String query)
-   {
-     List<String> ids = new EnsemblSymbol(getDomain(), getDbSource(),
-             getDbVersion()).getIds(query);
-     if (ids != null)
-     {
-       for (String id : ids)
-       {
-         if (isGeneIdentifier(id))
-         {
-           return id;
-         }
-       }
-     }
-     return null;
-   }
-   /**
     * Constructs all transcripts for the gene, as identified by "transcript"
     * features whose Parent is the requested gene. The coding transcript
     * sequences (i.e. with introns omitted) are added to the alignment.
      cdna.transferFeatures(gene.getFeatures().getPositionalFeatures(),
              transcript.getDatasetSequence(), mapping, parentId);
  
 +    mapTranscriptToChromosome(transcript, gene, mapping);
 +
      /*
       * fetch and save cross-references
       */
    }
  
    /**
 +   * If the gene has a mapping to chromosome coordinates, derive the transcript
 +   * chromosome regions and save on the transcript sequence
 +   * 
 +   * @param transcript
 +   * @param gene
 +   * @param mapping
 +   *          the mapping from gene to transcript positions
 +   */
 +  protected void mapTranscriptToChromosome(SequenceI transcript,
 +          SequenceI gene, MapList mapping)
 +  {
 +    GeneLociI loci = gene.getGeneLoci();
 +    if (loci == null)
 +    {
 +      return;
 +    }
 +
 +    MapList geneMapping = loci.getMap();
 +
 +    List<int[]> exons = mapping.getFromRanges();
 +    List<int[]> transcriptLoci = new ArrayList<>();
 +
 +    for (int[] exon : exons)
 +    {
 +      transcriptLoci.add(geneMapping.locateInTo(exon[0], exon[1]));
 +    }
 +
 +    List<int[]> transcriptRange = Arrays.asList(new int[] {
 +        transcript.getStart(), transcript.getEnd() });
 +    MapList mapList = new MapList(transcriptRange, transcriptLoci, 1, 1);
 +
 +    transcript.setGeneLoci(loci.getSpeciesId(), loci.getAssemblyId(),
 +            loci.getChromosomeId(), mapList);
 +  }
 +
 +  /**
     * Returns the 'transcript_id' property of the sequence feature (or null)
     * 
     * @param feature
@@@ -28,24 -28,28 +28,29 @@@ import java.net.MalformedURLException
  import java.net.URL;
  import java.util.Arrays;
  import java.util.List;
++import java.util.function.Function;
  
  import org.json.simple.JSONObject;
  import org.json.simple.parser.JSONParser;
  import org.json.simple.parser.ParseException;
  
  /**
-- * A client for the Ensembl lookup REST endpoint; used to find the Parent gene
-- * identifier given a transcript identifier.
++ * A client for the Ensembl lookup REST endpoint
   * 
   * @author gmcarstairs
   */
  public class EnsemblLookup extends EnsemblRestClient
  {
 +  private static final String SPECIES = "species";
  
 -  private static final String OBJECT_TYPE_TRANSLATION = "Translation";
    private static final String PARENT = "Parent";
 +
++  private static final String OBJECT_TYPE_TRANSLATION = "Translation";
+   private static final String OBJECT_TYPE_TRANSCRIPT = "Transcript";
+   private static final String ID = "id";
+   private static final String OBJECT_TYPE_GENE = "Gene";
+   private static final String OBJECT_TYPE = "object_type";
    /**
     * Default constructor (to use rest.ensembl.org)
     */
@@@ -90,7 -94,7 +95,7 @@@
    protected URL getUrl(String identifier)
    {
      String url = getDomain() + "/lookup/id/" + identifier
-             + "?content-type=application/json";
+             + CONTENT_TYPE_JSON;
      try
      {
        return new URL(url);
     * @param identifier
     * @return
     */
-   public String getParent(String identifier)
+   public String getGeneId(String identifier)
    {
-     return getAttribute(identifier, PARENT);
++    return getResult(identifier, br -> parseGeneId(br));
 +  }
 +
 +  /**
 +   * Calls the Ensembl lookup REST endpoint and retrieves the 'species' for the
 +   * given identifier, or null if not found
 +   * 
 +   * @param identifier
 +   * @return
 +   */
 +  public String getSpecies(String identifier)
 +  {
-     return getAttribute(identifier, SPECIES);
++    return getResult(identifier, br -> getAttribute(br, SPECIES));
 +  }
 +
 +  /**
 +   * @param identifier
 +   * @param attribute
 +   * @return
 +   */
-   protected String getAttribute(String identifier, String attribute)
++  protected String getResult(String identifier,
++          Function<BufferedReader, String> parser)
 +  {
      List<String> ids = Arrays.asList(new String[] { identifier });
  
      BufferedReader br = null;
        {
          br = getHttpResponse(url, ids);
        }
-       return (parseResponse(br, attribute));
 -      return br == null ? null : parseResponse(br);
++      return br == null ? null : parser.apply(br);
      } catch (IOException e)
      {
        // ignore
    }
  
    /**
-    * Parses the value of 'attribute' from the JSON response and returns the
-    * value, or null if not found
++   * Answers the value of 'attribute' from the JSON response, or null if not
++   * found
 +   * 
 +   * @param br
 +   * @param attribute
 +   * @return
-    * @throws IOException
 +   */
-   protected String parseResponse(BufferedReader br, String attribute) throws IOException
++  protected String getAttribute(BufferedReader br, String attribute)
++  {
++    String value = null;
++    JSONParser jp = new JSONParser();
++    try
++    {
++      JSONObject val = (JSONObject) jp.parse(br);
++      value = val.get(attribute).toString();
++    } catch (ParseException | NullPointerException | IOException e)
++    {
++      // ignore
++    }
++    return value;
++  }
++
++  /**
+    * Parses the JSON response and returns the gene identifier, or null if not
+    * found. If the returned object_type is Gene, returns the id, if Transcript
+    * returns the Parent. If it is Translation (peptide identifier), then the
+    * Parent is the transcript identifier, so we redo the search with this value.
+    * 
+    * @param br
+    * @return
 -   * @throws IOException
+    */
 -  protected String parseResponse(BufferedReader br) throws IOException
++  protected String parseGeneId(BufferedReader br)
    {
-     String parent = null;
+     String geneId = null;
      JSONParser jp = new JSONParser();
      try
      {
        JSONObject val = (JSONObject) jp.parse(br);
-       parent = val.get(attribute).toString();
-     } catch (ParseException | NullPointerException e)
+       String type = val.get(OBJECT_TYPE).toString();
+       if (OBJECT_TYPE_GENE.equalsIgnoreCase(type))
+       {
+         geneId = val.get(ID).toString();
+       }
+       else if (OBJECT_TYPE_TRANSCRIPT.equalsIgnoreCase(type))
+       {
+         geneId = val.get(PARENT).toString();
+       }
+       else if (OBJECT_TYPE_TRANSLATION.equalsIgnoreCase(type))
+       {
+         String transcriptId = val.get(PARENT).toString();
+         try
+         {
+           geneId = getGeneId(transcriptId);
+         } catch (StackOverflowError e)
+         {
+           /*
+            * unlikely data condition error!
+            */
+           System.err
+                   .println("** Ensembl lookup "
+                           + getUrl(transcriptId).toString()
+                           + " looping on Parent!");
+         }
+       }
 -    } catch (ParseException e)
++    } catch (ParseException | IOException e)
      {
        // ignore
      }
-     return parent;
+     return geneId;
    }
  
  }
@@@ -31,6 -31,7 +31,7 @@@ import java.io.InputStream
  import java.io.InputStreamReader;
  import java.net.HttpURLConnection;
  import java.net.MalformedURLException;
+ import java.net.ProtocolException;
  import java.net.URL;
  import java.util.HashMap;
  import java.util.List;
@@@ -42,8 -43,6 +43,6 @@@ import org.json.simple.JSONArray
  import org.json.simple.JSONObject;
  import org.json.simple.parser.JSONParser;
  
- import com.stevesoft.pat.Regex;
  /**
   * Base class for Ensembl REST service clients
   * 
@@@ -55,19 -54,25 +54,25 @@@ abstract class EnsemblRestClient extend
  
    private static final int CONNECT_TIMEOUT_MS = 10 * 1000; // 10 seconds
  
+   private static final int MAX_RETRIES = 3;
+   private static final int HTTP_OK = 200;
+   private static final int HTTP_OVERLOAD = 429;
    /*
     * update these constants when Jalview has been checked / updated for
     * changes to Ensembl REST API (ref JAL-2105)
     * @see https://github.com/Ensembl/ensembl-rest/wiki/Change-log
     * @see http://rest.ensembl.org/info/rest?content-type=application/json
     */
-   private static final String LATEST_ENSEMBLGENOMES_REST_VERSION = "5.0";
+   private static final String LATEST_ENSEMBLGENOMES_REST_VERSION = "6.0";
  
-   private static final String LATEST_ENSEMBL_REST_VERSION = "5.0";
+   private static final String LATEST_ENSEMBL_REST_VERSION = "6.1";
  
    private static final String REST_CHANGE_LOG = "https://github.com/Ensembl/ensembl-rest/wiki/Change-log";
  
 -  private static Map<String, EnsemblInfo> domainData;
 +  private static Map<String, EnsemblData> domainData;
  
    // @see https://github.com/Ensembl/ensembl-rest/wiki/Output-formats
    private static final String PING_URL = "http://rest.ensembl.org/info/ping.json";
  
    private final static long VERSION_RETEST_INTERVAL = 1000L * 3600; // 1 hr
  
-   private static final Regex PROTEIN_REGEX = new Regex(
-           "(ENS)([A-Z]{3}|)P[0-9]{11}$");
-   private static final Regex TRANSCRIPT_REGEX = new Regex(
-           "(ENS)([A-Z]{3}|)T[0-9]{11}$");
-   private static final Regex GENE_REGEX = new Regex(
-           "(ENS)([A-Z]{3}|)G[0-9]{11}$");
+   protected static final String CONTENT_TYPE_JSON = "?content-type=application/json";
  
    static
    {
-     domainData = new HashMap<String, EnsemblData>();
+     domainData = new HashMap<>();
      domainData.put(ENSEMBL_REST,
 -            new EnsemblInfo(ENSEMBL_REST, LATEST_ENSEMBL_REST_VERSION));
 -    domainData.put(ENSEMBL_GENOMES_REST, new EnsemblInfo(
 +            new EnsemblData(ENSEMBL_REST, LATEST_ENSEMBL_REST_VERSION));
 +    domainData.put(ENSEMBL_GENOMES_REST, new EnsemblData(
              ENSEMBL_GENOMES_REST, LATEST_ENSEMBLGENOMES_REST_VERSION));
    }
  
      setDomain(d);
    }
  
-   /**
-    * Answers true if the query matches the regular expression pattern for an
-    * Ensembl transcript stable identifier
-    * 
-    * @param query
-    * @return
-    */
-   public boolean isTranscriptIdentifier(String query)
-   {
-     return query == null ? false : TRANSCRIPT_REGEX.search(query);
-   }
-   /**
-    * Answers true if the query matches the regular expression pattern for an
-    * Ensembl protein stable identifier
-    * 
-    * @param query
-    * @return
-    */
-   public boolean isProteinIdentifier(String query)
-   {
-     return query == null ? false : PROTEIN_REGEX.search(query);
-   }
-   /**
-    * Answers true if the query matches the regular expression pattern for an
-    * Ensembl gene stable identifier
-    * 
-    * @param query
-    * @return
-    */
-   public boolean isGeneIdentifier(String query)
-   {
-     return query == null ? false : GENE_REGEX.search(query);
-   }
    @Override
    public boolean queryInProgress()
    {
     * @see http://rest.ensembl.org/documentation/info/ping
     * @return
     */
-   private boolean checkEnsembl()
+   boolean checkEnsembl()
    {
      BufferedReader br = null;
      try
      {
        // note this format works for both ensembl and ensemblgenomes
        // info/ping.json works for ensembl only (March 2016)
-       URL ping = new URL(
-               getDomain() + "/info/ping?content-type=application/json");
+       URL ping = new URL(getDomain() + "/info/ping" + CONTENT_TYPE_JSON);
  
        /*
         * expect {"ping":1} if ok
         * if ping takes more than 2 seconds to respond, treat as if unavailable
         */
        br = getHttpResponse(ping, null, 2 * 1000);
+       if (br == null)
+       {
+         // error reponse status
+         return false;
+       }
        JSONParser jp = new JSONParser();
        JSONObject val = (JSONObject) jp.parse(br);
        String pingString = val.get("ping").toString();
    }
  
    /**
-    * Writes the HTTP request and gets the response as a reader.
+    * Sends the HTTP request and gets the response as a reader
     * 
     * @param url
     * @param ids
    protected BufferedReader getHttpResponse(URL url, List<String> ids,
            int readTimeout) throws IOException
    {
-     // long now = System.currentTimeMillis();
+     int retriesLeft = MAX_RETRIES;
+     HttpURLConnection connection = null;
+     int responseCode = 0;
+     while (retriesLeft > 0)
+     {
+       connection = tryConnection(url, ids, readTimeout);
+       responseCode = connection.getResponseCode();
+       if (responseCode == HTTP_OVERLOAD) // 429
+       {
+         retriesLeft--;
+         checkRetryAfter(connection);
+       }
+       else
+       {
+         retriesLeft = 0;
+       }
+     }
+     if (responseCode != HTTP_OK) // 200
+     {
+       /*
+        * note: a GET request for an invalid id returns an error code e.g. 415
+        * but POST request returns 200 and an empty Fasta response 
+        */
+       System.err.println("Response code " + responseCode + " for " + url);
+       return null;
+     }
+     InputStream response = connection.getInputStream();
+     // System.out.println(getClass().getName() + " took "
+     // + (System.currentTimeMillis() - now) + "ms to fetch");
+     BufferedReader reader = null;
+     reader = new BufferedReader(new InputStreamReader(response, "UTF-8"));
+     return reader;
+   }
+   /**
+    * @param url
+    * @param ids
+    * @param readTimeout
+    * @return
+    * @throws IOException
+    * @throws ProtocolException
+    */
+   protected HttpURLConnection tryConnection(URL url, List<String> ids,
+           int readTimeout) throws IOException, ProtocolException
+   {
+     // System.out.println(System.currentTimeMillis() + " " + url);
      HttpURLConnection connection = (HttpURLConnection) url.openConnection();
  
      /*
      {
        writePostBody(connection, ids);
      }
-     int responseCode = connection.getResponseCode();
-     if (responseCode != 200)
-     {
-       /*
-        * note: a GET request for an invalid id returns an error code e.g. 415
-        * but POST request returns 200 and an empty Fasta response 
-        */
-       System.err.println("Response code " + responseCode + " for " + url);
-       return null;
-     }
-     // get content
-     InputStream response = connection.getInputStream();
-     // System.out.println(getClass().getName() + " took "
-     // + (System.currentTimeMillis() - now) + "ms to fetch");
-     checkRateLimits(connection);
-     BufferedReader reader = null;
-     reader = new BufferedReader(new InputStreamReader(response, "UTF-8"));
-     return reader;
+     return connection;
    }
  
    /**
-    * Inspect response headers for any sign of server overload and respect any
-    * 'retry-after' directive
+    * Inspects response headers for a 'retry-after' directive, and waits for the
+    * directed period (if less than 10 seconds)
     * 
     * @see https://github.com/Ensembl/ensembl-rest/wiki/Rate-Limits
     * @param connection
     */
-   void checkRateLimits(HttpURLConnection connection)
+   void checkRetryAfter(HttpURLConnection connection)
    {
-     // number of requests allowed per time interval:
-     String limit = connection.getHeaderField("X-RateLimit-Limit");
-     // length of quota time interval in seconds:
-     // String period = connection.getHeaderField("X-RateLimit-Period");
-     // seconds remaining until usage quota is reset:
-     String reset = connection.getHeaderField("X-RateLimit-Reset");
-     // number of requests remaining from quota for current period:
-     String remaining = connection.getHeaderField("X-RateLimit-Remaining");
-     // number of seconds to wait before retrying (if remaining == 0)
      String retryDelay = connection.getHeaderField("Retry-After");
  
      // to test:
      // retryDelay = "5";
  
-     EnsemblData info = domainData.get(getDomain());
      if (retryDelay != null)
      {
-       System.err.println("Ensembl REST service rate limit exceeded, wait "
-               + retryDelay + " seconds before retrying");
        try
        {
-         info.retryAfter = System.currentTimeMillis()
-                 + (1000 * Integer.valueOf(retryDelay));
-       } catch (NumberFormatException e)
+         int retrySecs = Integer.valueOf(retryDelay);
+         if (retrySecs > 0 && retrySecs < 10)
+         {
+           System.err
+                   .println("Ensembl REST service rate limit exceeded, waiting "
+                           + retryDelay + " seconds before retrying");
+           Thread.sleep(1000 * retrySecs);
+         }
+       } catch (NumberFormatException | InterruptedException e)
        {
-         System.err
-                 .println("Unexpected value for Retry-After: " + retryDelay);
+         System.err.println("Error handling Retry-After: " + e.getMessage());
        }
      }
-     else
-     {
-       info.retryAfter = 0;
-       // debug:
-       // System.out.println(String.format(
-       // "%s Ensembl requests remaining of %s (reset in %ss)",
-       // remaining, limit, reset));
-     }
    }
  
    /**
     */
    protected boolean isEnsemblAvailable()
    {
 -    EnsemblInfo info = domainData.get(getDomain());
 +    EnsemblData info = domainData.get(getDomain());
  
      long now = System.currentTimeMillis();
  
      /*
-      * check if we are waiting for 'Retry-After' to expire
-      */
-     if (info.retryAfter > now)
-     {
-       System.err.println("Still " + (1 + (info.retryAfter - now) / 1000)
-               + " secs to wait before retrying Ensembl");
-       return false;
-     }
-     else
-     {
-       info.retryAfter = 0;
-     }
-     /*
       * recheck if Ensembl is up if it was down, or the recheck period has elapsed
       */
      boolean retestAvailability = (now
     */
    private void checkEnsemblRestVersion()
    {
 -    EnsemblInfo info = domainData.get(getDomain());
 +    EnsemblData info = domainData.get(getDomain());
  
      JSONParser jp = new JSONParser();
      URL url = null;
      try
      {
-       url = new URL(
-               getDomain() + "/info/rest?content-type=application/json");
+       url = new URL(getDomain() + "/info/rest" + CONTENT_TYPE_JSON);
        BufferedReader br = getHttpResponse(url, null);
+       if (br == null)
+       {
+         return;
+       }
        JSONObject val = (JSONObject) jp.parse(br);
        String version = val.get("release").toString();
        String majorVersion = version.substring(0, version.indexOf("."));
    {
      JSONParser jp = new JSONParser();
      URL url = null;
+     BufferedReader br = null;
      try
      {
-       url = new URL(
-               getDomain() + "/info/data?content-type=application/json");
-       BufferedReader br = getHttpResponse(url, null);
-       JSONObject val = (JSONObject) jp.parse(br);
-       JSONArray versions = (JSONArray) val.get("releases");
-       domainData.get(getDomain()).dataVersion = versions.get(0).toString();
+       url = new URL(getDomain() + "/info/data" + CONTENT_TYPE_JSON);
+       br = getHttpResponse(url, null);
+       if (br != null)
+       {
+         JSONObject val = (JSONObject) jp.parse(br);
+         JSONArray versions = (JSONArray) val.get("releases");
+         domainData.get(getDomain()).dataVersion = versions.get(0)
+                 .toString();
+       }
      } catch (Throwable t)
      {
        System.err.println(
                "Error checking Ensembl data version: " + t.getMessage());
+     } finally
+     {
+       if (br != null)
+       {
+         try
+         {
+           br.close();
+         } catch (IOException e)
+         {
+           // ignore
+         }
+       }
      }
    }
  
@@@ -81,7 -81,6 +81,7 @@@ import jalview.io.JnetAnnotationMaker
  import jalview.io.NewickFile;
  import jalview.io.ScoreMatrixFile;
  import jalview.io.TCoffeeScoreFile;
 +import jalview.io.vcf.VCFLoader;
  import jalview.jbgui.GAlignFrame;
  import jalview.schemes.ColourSchemeI;
  import jalview.schemes.ColourSchemes;
@@@ -842,7 -841,6 +842,7 @@@ public class AlignFrame extends GAlignF
      AlignmentI al = getViewport().getAlignment();
      boolean nucleotide = al.isNucleotide();
  
 +    loadVcf.setVisible(nucleotide);
      showTranslation.setVisible(nucleotide);
      showReverse.setVisible(nucleotide);
      showReverseComplement.setVisible(nucleotide);
      }
      viewport.getAlignment().moveSelectedSequencesByOne(sg,
              viewport.getHiddenRepSequences(), up);
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, false);
    }
  
    synchronized void slideSequences(boolean right, int size)
      {
        PaintRefresher.Refresh(this, viewport.getSequenceSetId());
        alignPanel.updateAnnotation();
-       alignPanel.paintAlignment(true);
+       alignPanel.paintAlignment(true, true);
      }
    }
  
      // JAL-2034 - should delegate to
      // alignPanel to decide if overview needs
      // updating.
-     alignPanel.paintAlignment(false);
+     alignPanel.paintAlignment(false, false);
      PaintRefresher.Refresh(alignPanel, viewport.getSequenceSetId());
    }
  
      // JAL-2034 - should delegate to
      // alignPanel to decide if overview needs
      // updating.
-     alignPanel.paintAlignment(false);
+     alignPanel.paintAlignment(false, false);
      PaintRefresher.Refresh(alignPanel, viewport.getSequenceSetId());
      viewport.sendSelection();
    }
      // alignPanel to decide if overview needs
      // updating.
  
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, false);
      PaintRefresher.Refresh(alignPanel, viewport.getSequenceSetId());
      viewport.sendSelection();
    }
    public void invertColSel_actionPerformed(ActionEvent e)
    {
      viewport.invertColumnSelection();
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, false);
      viewport.sendSelection();
    }
  
  
      alignPanel.getIdPanel().getIdCanvas()
              .setPreferredSize(alignPanel.calculateIdWidth());
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, false);
    }
  
    @Override
    public void idRightAlign_actionPerformed(ActionEvent e)
    {
      viewport.setRightAlignIds(idRightAlign.isSelected());
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(false, false);
    }
  
    @Override
    public void centreColumnLabels_actionPerformed(ActionEvent e)
    {
      viewport.setCentreColumnLabels(centreColumnLabelsMenuItem.getState());
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(false, false);
    }
  
    /*
    protected void colourTextMenuItem_actionPerformed(ActionEvent e)
    {
      viewport.setColourText(colourTextMenuItem.isSelected());
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(false, false);
    }
  
    /**
    public void showAllColumns_actionPerformed(ActionEvent e)
    {
      viewport.showAllHiddenColumns();
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, true);
      viewport.sendSelection();
    }
  
      viewport.expandColSelection(sg, false);
      viewport.hideAllSelectedSeqs();
      viewport.hideSelectedColumns();
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, true);
      viewport.sendSelection();
    }
  
    {
      viewport.showAllHiddenColumns();
      viewport.showAllHiddenSeqs();
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, true);
      viewport.sendSelection();
    }
  
    public void hideSelColumns_actionPerformed(ActionEvent e)
    {
      viewport.hideSelectedColumns();
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, true);
      viewport.sendSelection();
    }
  
    protected void scaleAbove_actionPerformed(ActionEvent e)
    {
      viewport.setScaleAboveWrapped(scaleAbove.isSelected());
-     alignPanel.paintAlignment(true);
+     // TODO: do we actually need to update overview for scale above change ?
+     alignPanel.paintAlignment(true, false);
    }
  
    /**
    protected void scaleLeft_actionPerformed(ActionEvent e)
    {
      viewport.setScaleLeftWrapped(scaleLeft.isSelected());
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, false);
    }
  
    /**
    protected void scaleRight_actionPerformed(ActionEvent e)
    {
      viewport.setScaleRightWrapped(scaleRight.isSelected());
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, false);
    }
  
    /**
    public void viewBoxesMenuItem_actionPerformed(ActionEvent e)
    {
      viewport.setShowBoxes(viewBoxesMenuItem.isSelected());
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(false, false);
    }
  
    /**
    public void viewTextMenuItem_actionPerformed(ActionEvent e)
    {
      viewport.setShowText(viewTextMenuItem.isSelected());
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(false, false);
    }
  
    /**
    protected void renderGapsMenuItem_actionPerformed(ActionEvent e)
    {
      viewport.setRenderGaps(renderGapsMenuItem.isSelected());
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(false, false);
    }
  
    public FeatureSettings featureSettings;
    public void showSeqFeatures_actionPerformed(ActionEvent evt)
    {
      viewport.setShowSequenceFeatures(showSeqFeatures.isSelected());
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, true);
    }
  
    /**
  
      viewport.setGlobalColourScheme(cs);
  
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, true);
    }
  
    /**
              viewport.getAlignment().getSequenceAt(0));
      addHistoryItem(new OrderCommand("Pairwise Sort", oldOrder,
              viewport.getAlignment()));
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, false);
    }
  
    /**
      AlignmentSorter.sortByID(viewport.getAlignment());
      addHistoryItem(
              new OrderCommand("ID Sort", oldOrder, viewport.getAlignment()));
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, false);
    }
  
    /**
      AlignmentSorter.sortByLength(viewport.getAlignment());
      addHistoryItem(new OrderCommand("Length Sort", oldOrder,
              viewport.getAlignment()));
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, false);
    }
  
    /**
      addHistoryItem(new OrderCommand("Group Sort", oldOrder,
              viewport.getAlignment()));
  
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, false);
    }
  
    /**
          addHistoryItem(new OrderCommand(order.getName(), oldOrder,
                  viewport.getAlignment()));
  
-         alignPanel.paintAlignment(true);
+         alignPanel.paintAlignment(true, false);
        }
      });
    }
                  viewport.getAlignment());// ,viewport.getSelectionGroup());
          addHistoryItem(new OrderCommand("Sort by " + scoreLabel, oldOrder,
                  viewport.getAlignment()));
-         alignPanel.paintAlignment(true);
+         alignPanel.paintAlignment(true, false);
        }
      });
    }
        addHistoryItem(new OrderCommand(undoname, oldOrder,
                viewport.getAlignment()));
      }
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(true, false);
      return true;
    }
  
    protected void showProductsFor(final SequenceI[] sel, final boolean _odna,
            final String source)
    {
 -    new Thread(CrossRefAction.showProductsFor(sel, _odna, source, this))
 +    new Thread(CrossRefAction.getHandlerFor(sel, _odna, source, this))
              .start();
    }
  
                        assocfiles++;
                      }
                    }
-                   alignPanel.paintAlignment(true);
+                   // TODO: do we need to update overview ? only if features are
+                   // shown I guess
+                   alignPanel.paintAlignment(true, false);
                  }
                }
              }
            {
              if (parseFeaturesFile(file, sourceType))
              {
-               alignPanel.paintAlignment(true);
+               alignPanel.paintAlignment(true, true);
              }
            }
            else
          alignPanel.adjustAnnotationHeight();
          viewport.updateSequenceIdColours();
          buildSortByAnnotationScoresMenu();
-         alignPanel.paintAlignment(true);
+         alignPanel.paintAlignment(true, true);
        }
      } catch (Exception ex)
      {
    protected void showUnconservedMenuItem_actionPerformed(ActionEvent e)
    {
      viewport.setShowUnconserved(showNonconservedMenuItem.getState());
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(false, false);
    }
  
    /*
      {
        PaintRefresher.Refresh(this, viewport.getSequenceSetId());
        alignPanel.updateAnnotation();
-       alignPanel.paintAlignment(true);
+       alignPanel.paintAlignment(true, true);
      }
    }
  
        viewport.getAlignment().setSeqrep(null);
        PaintRefresher.Refresh(this, viewport.getSequenceSetId());
        alignPanel.updateAnnotation();
-       alignPanel.paintAlignment(true);
+       alignPanel.paintAlignment(true, true);
      }
    }
  
      this.alignPanel.av.setSortAnnotationsBy(getAnnotationSortOrder());
      this.alignPanel.av
              .setShowAutocalculatedAbove(isShowAutoCalculatedAbove());
-     alignPanel.paintAlignment(true);
+     alignPanel.paintAlignment(false, false);
    }
  
    /**
        new CalculationChooser(AlignFrame.this);
      }
    }
 +
 +  @Override
 +  protected void loadVcf_actionPerformed()
 +  {
 +    JalviewFileChooser chooser = new JalviewFileChooser(
 +            Cache.getProperty("LAST_DIRECTORY"));
 +    chooser.setFileView(new JalviewFileView());
 +    chooser.setDialogTitle(MessageManager.getString("label.load_vcf_file"));
 +    chooser.setToolTipText(MessageManager.getString("label.load_vcf_file"));
 +
 +    int value = chooser.showOpenDialog(null);
 +
 +    if (value == JalviewFileChooser.APPROVE_OPTION)
 +    {
 +      String choice = chooser.getSelectedFile().getPath();
 +      Cache.setProperty("LAST_DIRECTORY", choice);
 +      new VCFLoader(viewport.getAlignment()).loadVCF(choice, this);
 +    }
 +
 +  }
  }
  
  class PrintThread extends Thread
@@@ -138,7 -138,7 +138,7 @@@ public class IdPanel extends JPane
      }
  
      lastid = seq;
-     alignPanel.paintAlignment(false);
+     alignPanel.paintAlignment(false, false);
    }
  
    /**
  
      av.isSelectionGroupChanged(true);
  
-     alignPanel.paintAlignment(false);
+     alignPanel.paintAlignment(false, false);
    }
  
    /**
       *  and any non-positional features
       */
      List<String> nlinks = Preferences.sequenceUrlLinks.getLinksForMenu();
 -    for (SequenceFeature sf : sq.getFeatures().getNonPositionalFeatures())
 +    List<SequenceFeature> features = sq.getFeatures().getNonPositionalFeatures();
 +    for (SequenceFeature sf : features)
      {
        if (sf.links != null)
        {
        }
      }
  
 -    PopupMenu pop = new PopupMenu(alignPanel, sq, nlinks,
 +    PopupMenu pop = new PopupMenu(alignPanel, sq, features,
              Preferences.getGroupURLLinks());
      pop.show(this, e.getX(), e.getY());
    }
            running = false;
          }
  
-         alignPanel.paintAlignment(false);
+         alignPanel.paintAlignment(false, false);
  
          try
          {
@@@ -34,6 -34,7 +34,6 @@@ import jalview.datamodel.Annotation
  import jalview.datamodel.DBRefEntry;
  import jalview.datamodel.HiddenColumns;
  import jalview.datamodel.PDBEntry;
 -import jalview.datamodel.Sequence;
  import jalview.datamodel.SequenceFeature;
  import jalview.datamodel.SequenceGroup;
  import jalview.datamodel.SequenceI;
@@@ -175,31 -176,25 +175,31 @@@ public class PopupMenu extends JPopupMe
     * Creates a new PopupMenu object.
     * 
     * @param ap
 -   *          DOCUMENT ME!
     * @param seq
 -   *          DOCUMENT ME!
 +   * @param features
 +   *          non-positional features (for seq not null), or positional features
 +   *          at residue (for seq equal to null)
     */
 -  public PopupMenu(final AlignmentPanel ap, Sequence seq,
 -          List<String> links)
 +  public PopupMenu(final AlignmentPanel ap, SequenceI seq,
 +          List<SequenceFeature> features)
    {
 -    this(ap, seq, links, null);
 +    this(ap, seq, features, null);
    }
  
    /**
 +   * Constructor
     * 
 -   * @param ap
 +   * @param alignPanel
     * @param seq
 -   * @param links
 +   *          the sequence under the cursor if in the Id panel, null if in the
 +   *          sequence panel
 +   * @param features
 +   *          non-positional features if in the Id panel, features at the
 +   *          clicked residue if in the sequence panel
     * @param groupLinks
     */
 -  public PopupMenu(final AlignmentPanel ap, final SequenceI seq,
 -          List<String> links, List<String> groupLinks)
 +  public PopupMenu(final AlignmentPanel alignPanel, final SequenceI seq,
 +          List<SequenceFeature> features, List<String> groupLinks)
    {
      // /////////////////////////////////////////////////////////
      // If this is activated from the sequence panel, the user may want to
      //
      // If from the IDPanel, we must display the sequence menu
      // ////////////////////////////////////////////////////////
 -    this.ap = ap;
 +    this.ap = alignPanel;
      sequence = seq;
  
      for (String ff : FileFormats.getInstance().getWritableFormats(true))
      /*
       * And repeat for the current selection group (if there is one):
       */
 -    final List<SequenceI> selectedGroup = (ap.av.getSelectionGroup() == null
 +    final List<SequenceI> selectedGroup = (alignPanel.av.getSelectionGroup() == null
              ? Collections.<SequenceI> emptyList()
 -            : ap.av.getSelectionGroup().getSequences());
 +            : alignPanel.av.getSelectionGroup().getSequences());
      buildAnnotationTypesMenus(groupShowAnnotationsMenu,
              groupHideAnnotationsMenu, selectedGroup);
      configureReferenceAnnotationsMenu(groupAddReferenceAnnotations,
      if (seq != null)
      {
        sequenceMenu.setText(sequence.getName());
 -      if (seq == ap.av.getAlignment().getSeqrep())
 +      if (seq == alignPanel.av.getAlignment().getSeqrep())
        {
          makeReferenceSeq.setText(
                  MessageManager.getString("action.unmark_as_reference"));
                  MessageManager.getString("action.set_as_reference"));
        }
  
 -      if (!ap.av.getAlignment().isNucleotide())
 +      if (!alignPanel.av.getAlignment().isNucleotide())
        {
          remove(rnaStructureMenu);
        }
           * add menu items to 2D-render any alignment or sequence secondary
           * structure annotation
           */
 -        AlignmentAnnotation[] aas = ap.av.getAlignment()
 +        AlignmentAnnotation[] aas = alignPanel.av.getAlignment()
                  .getAlignmentAnnotation();
          if (aas != null)
          {
                  @Override
                  public void actionPerformed(ActionEvent e)
                  {
 -                  new AppVarna(seq, aa, ap);
 +                  new AppVarna(seq, aa, alignPanel);
                  }
                });
                rnaStructureMenu.add(menuItem);
                  public void actionPerformed(ActionEvent e)
                  {
                    // TODO: VARNA does'nt print gaps in the sequence
 -                  new AppVarna(seq, aa, ap);
 +                  new AppVarna(seq, aa, alignPanel);
                  }
                });
                rnaStructureMenu.add(menuItem);
        });
        add(menuItem);
  
 -      if (ap.av.getSelectionGroup() != null
 -              && ap.av.getSelectionGroup().getSize() > 1)
 +      if (alignPanel.av.getSelectionGroup() != null
 +              && alignPanel.av.getSelectionGroup().getSize() > 1)
        {
          menuItem = new JMenuItem(MessageManager
                  .formatMessage("label.represent_group_with", new Object[]
          sequenceMenu.add(menuItem);
        }
  
 -      if (ap.av.hasHiddenRows())
 +      if (alignPanel.av.hasHiddenRows())
        {
 -        final int index = ap.av.getAlignment().findIndex(seq);
 +        final int index = alignPanel.av.getAlignment().findIndex(seq);
  
 -        if (ap.av.adjustForHiddenSeqs(index)
 -                - ap.av.adjustForHiddenSeqs(index - 1) > 1)
 +        if (alignPanel.av.adjustForHiddenSeqs(index)
 +                - alignPanel.av.adjustForHiddenSeqs(index - 1) > 1)
          {
            menuItem = new JMenuItem(
                    MessageManager.getString("action.reveal_sequences"));
              @Override
              public void actionPerformed(ActionEvent e)
              {
 -              ap.av.showSequence(index);
 -              if (ap.overviewPanel != null)
 +              alignPanel.av.showSequence(index);
 +              if (alignPanel.overviewPanel != null)
                {
 -                ap.overviewPanel.updateOverviewImage();
 +                alignPanel.overviewPanel.updateOverviewImage();
                }
              }
            });
        }
      }
      // for the case when no sequences are even visible
 -    if (ap.av.hasHiddenRows())
 +    if (alignPanel.av.hasHiddenRows())
      {
        {
          menuItem = new JMenuItem(
            @Override
            public void actionPerformed(ActionEvent e)
            {
 -            ap.av.showAllHiddenSeqs();
 -            if (ap.overviewPanel != null)
 +            alignPanel.av.showAllHiddenSeqs();
 +            if (alignPanel.overviewPanel != null)
              {
 -              ap.overviewPanel.updateOverviewImage();
 +              alignPanel.overviewPanel.updateOverviewImage();
              }
            }
          });
        }
      }
  
 -    SequenceGroup sg = ap.av.getSelectionGroup();
 +    SequenceGroup sg = alignPanel.av.getSelectionGroup();
      boolean isDefinedGroup = (sg != null)
 -            ? ap.av.getAlignment().getGroups().contains(sg)
 +            ? alignPanel.av.getAlignment().getGroups().contains(sg)
              : false;
  
      if (sg != null && sg.getSize() > 0)
        Hashtable<String, PDBEntry> pdbe = new Hashtable<>(), reppdb = new Hashtable<>();
  
        SequenceI sqass = null;
 -      for (SequenceI sq : ap.av.getSequenceSelection())
 +      for (SequenceI sq : alignPanel.av.getSequenceSelection())
        {
          Vector<PDBEntry> pes = sq.getDatasetSequence().getAllPDBEntries();
          if (pes != null && pes.size() > 0)
        rnaStructureMenu.setVisible(false);
      }
  
 -    if (links != null && links.size() > 0)
 +    addLinks(seq, features);
 +
 +    if (seq == null)
 +    {
 +      addFeatureDetails(features);
 +    }
 +  }
 +
 +  /**
 +   * Add a link to show feature details for each sequence feature
 +   * 
 +   * @param features
 +   */
 +  protected void addFeatureDetails(List<SequenceFeature> features)
 +  {
 +    if (features == null || features.isEmpty())
 +    {
 +      return;
 +    }
 +    JMenu details = new JMenu(
 +            MessageManager.getString("label.feature_details"));
 +    add(details);
 +
 +    for (final SequenceFeature sf : features)
      {
 -      addFeatureLinks(seq, links);
 +      int start = sf.getBegin();
 +      int end = sf.getEnd();
 +      String desc = null;
 +      if (start == end)
 +      {
 +        desc = String.format("%s %d", sf.getType(), start);
 +      }
 +      else
 +      {
 +        desc = String.format("%s %d-%d", sf.getType(), start, end);
 +      }
 +      String description = sf.getDescription();
 +      if (description != null)
 +      {
 +        if (description.length() <= 6)
 +        {
 +          desc = desc + " " + description;
 +        }
 +        else
 +        {
 +          desc = desc + " " + description.substring(0, 6) + "..";
 +        }
 +      }
 +      if (sf.getFeatureGroup() != null)
 +      {
 +        desc = desc + " (" + sf.getFeatureGroup() + ")";
 +      }
 +      JMenuItem item = new JMenuItem(desc);
 +      item.addActionListener(new ActionListener()
 +      {
 +        @Override
 +        public void actionPerformed(ActionEvent e)
 +        {
 +          showFeatureDetails(sf);
 +        }
 +      });
 +      details.add(item);
      }
    }
  
    /**
 +   * Opens a panel showing a text report of feature dteails
 +   * 
 +   * @param sf
 +   */
 +  protected void showFeatureDetails(SequenceFeature sf)
 +  {
 +    CutAndPasteTransfer cap = new CutAndPasteTransfer();
 +    cap.setText(sf.getDetailsReport());
 +    Desktop.addInternalFrame(cap,
 +            MessageManager.getString("label.feature_details"), 500, 500);
 +  }
 +
 +  /**
     * Adds a 'Link' menu item with a sub-menu item for each hyperlink provided.
 +   * When seq is not null, these are links for the sequence id, which may be to
 +   * external web sites for the sequence accession, and/or links embedded in
 +   * non-positional features. When seq is null, only links embedded in the
 +   * provided features are added.
     * 
     * @param seq
 -   * @param links
 +   * @param features
     */
 -  void addFeatureLinks(final SequenceI seq, List<String> links)
 +  void addLinks(final SequenceI seq, List<SequenceFeature> features)
    {
      JMenu linkMenu = new JMenu(MessageManager.getString("action.link"));
 +
 +    List<String> nlinks = null;
 +    if (seq != null)
 +    {
 +      nlinks = Preferences.sequenceUrlLinks.getLinksForMenu();
 +    }
 +    else
 +    {
 +      nlinks = new ArrayList<>();
 +    }
 +
 +    if (features != null)
 +    {
 +      for (SequenceFeature sf : features)
 +      {
 +        if (sf.links != null)
 +        {
 +          for (String link : sf.links)
 +          {
 +            nlinks.add(link);
 +          }
 +        }
 +      }
 +    }
 +
      Map<String, List<String>> linkset = new LinkedHashMap<>();
  
 -    for (String link : links)
 +    for (String link : nlinks)
      {
        UrlLink urlLink = null;
        try
  
      addshowLinks(linkMenu, linkset.values());
  
 -    // disable link menu if there are no valid entries
 +    // only add link menu if it has entries
      if (linkMenu.getItemCount() > 0)
      {
 -      linkMenu.setEnabled(true);
 -    }
 -    else
 -    {
 -      linkMenu.setEnabled(false);
 -    }
 -
 -    if (sequence != null)
 -    {
 -      sequenceMenu.add(linkMenu);
 -    }
 -    else
 -    {
 -      add(linkMenu);
 +      if (sequence != null)
 +      {
 +        sequenceMenu.add(linkMenu);
 +      }
 +      else
 +      {
 +        add(linkMenu);
 +      }
      }
 -
    }
  
    /**
        }
  
        sequence.setName(dialog.getName().replace(' ', '_'));
-       ap.paintAlignment(false);
+       ap.paintAlignment(false, false);
      }
  
      sequence.setDescription(dialog.getDescription());
@@@ -59,6 -59,7 +59,6 @@@ import java.awt.event.MouseListener
  import java.awt.event.MouseMotionListener;
  import java.awt.event.MouseWheelEvent;
  import java.awt.event.MouseWheelListener;
 -import java.util.ArrayList;
  import java.util.Collections;
  import java.util.List;
  
@@@ -214,8 -215,8 +214,8 @@@ public class SeqPanel extends JPane
                + hgap + seqCanvas.getAnnotationHeight();
  
        int y = evt.getY();
-       y -= hgap;
-       x = Math.max(0, x - seqCanvas.labelWidthWest);
+       y = Math.max(0, y - hgap);
+       x = Math.max(0, x - seqCanvas.getLabelWidthWest());
  
        int cwidth = seqCanvas.getWrappedCanvasWidth(this.getWidth());
        if (cwidth < 1)
        av.setSelectionGroup(sg);
      }
  
-     ap.paintAlignment(false);
+     ap.paintAlignment(false, false);
      av.sendSelection();
    }
  
  
    /**
     * set when the current UI interaction has resulted in a change that requires
-    * overview shading to be recalculated. this could be changed to something
-    * more expressive that indicates what actually has changed, so selective
-    * redraws can be applied
+    * shading in overviews and structures to be recalculated. this could be
+    * changed to a something more expressive that indicates what actually has
+    * changed, so selective redraws can be applied (ie. only structures, only
+    * overview, etc)
     */
-   private boolean needOverviewUpdate = false; // TODO: refactor to avcontroller
+   private boolean updateOverviewAndStructs = false; // TODO: refactor to avcontroller
  
    /**
     * set if av.getSelectionGroup() refers to a group that is defined on the
          }
          if (newWidth > 0)
          {
-           ap.paintAlignment(false);
+           ap.paintAlignment(false, false);
            if (copyChanges)
            {
              /*
      final int res = findColumn(evt);
      final int seq = findSeq(evt);
      oldSeq = seq;
-     needOverviewUpdate = false;
+     updateOverviewAndStructs = false;
  
      startWrapBlock = wrappedBlock;
  
      final int column = findColumn(evt);
      final int seq = findSeq(evt);
      SequenceI sequence = av.getAlignment().getSequenceAt(seq);
 -    List<SequenceFeature> allFeatures = ap.getFeatureRenderer()
 +    List<SequenceFeature> features = ap.getFeatureRenderer()
              .findFeaturesAtColumn(sequence, column + 1);
 -    List<String> links = new ArrayList<>();
 -    for (SequenceFeature sf : allFeatures)
 -    {
 -      if (sf.links != null)
 -      {
 -        for (String link : sf.links)
 -        {
 -          links.add(link);
 -        }
 -      }
 -    }
  
 -    PopupMenu pop = new PopupMenu(ap, null, links);
 +    PopupMenu pop = new PopupMenu(ap, null, features);
      pop.show(this, evt.getX(), evt.getY());
    }
  
      // always do this - annotation has own state
      // but defer colourscheme update until hidden sequences are passed in
      boolean vischange = stretchGroup.recalcConservation(true);
-     needOverviewUpdate |= vischange && av.isSelectionDefinedGroup()
+     updateOverviewAndStructs |= vischange && av.isSelectionDefinedGroup()
              && afterDrag;
      if (stretchGroup.cs != null)
      {
        }
      }
      PaintRefresher.Refresh(this, av.getSequenceSetId());
-     ap.paintAlignment(needOverviewUpdate);
-     needOverviewUpdate = false;
+     // TODO: structure colours only need updating if stretchGroup used to or now
+     // does contain sequences with structure views
+     ap.paintAlignment(updateOverviewAndStructs, updateOverviewAndStructs);
+     updateOverviewAndStructs = false;
      changeEndRes = false;
      changeStartRes = false;
      stretchGroup = null;
        if (res > (stretchGroup.getStartRes() - 1))
        {
          stretchGroup.setEndRes(res);
-         needOverviewUpdate |= av.isSelectionDefinedGroup();
+         updateOverviewAndStructs |= av.isSelectionDefinedGroup();
        }
      }
      else if (changeStartRes)
        if (res < (stretchGroup.getEndRes() + 1))
        {
          stretchGroup.setStartRes(res);
-         needOverviewUpdate |= av.isSelectionDefinedGroup();
+         updateOverviewAndStructs |= av.isSelectionDefinedGroup();
        }
      }
  
        if (stretchGroup.getSequences(null).contains(nextSeq))
        {
          stretchGroup.deleteSequence(seq, false);
-         needOverviewUpdate |= av.isSelectionDefinedGroup();
+         updateOverviewAndStructs |= av.isSelectionDefinedGroup();
        }
        else
        {
          }
  
          stretchGroup.addSequence(nextSeq, false);
-         needOverviewUpdate |= av.isSelectionDefinedGroup();
+         updateOverviewAndStructs |= av.isSelectionDefinedGroup();
        }
      }
  
index 0000000,a27bc3f..05b9aea
mode 000000,100644..100644
--- /dev/null
@@@ -1,0 -1,283 +1,281 @@@
+ package jalview.gui;
+ import static org.testng.Assert.assertEquals;
+ import jalview.datamodel.AlignmentI;
+ import jalview.io.DataSourceType;
+ import jalview.io.FileLoader;
+ import java.awt.Font;
+ import java.awt.FontMetrics;
+ import junit.extensions.PA;
+ import org.testng.annotations.Test;
 -import sun.swing.SwingUtilities2;
 -
+ public class SeqCanvasTest
+ {
+   /**
+    * Test the method that computes wrapped width in residues, height of wrapped
+    * widths in pixels, and the number of widths visible
+    */
+   @Test(groups = "Functional")
+   public void testCalculateWrappedGeometry_noAnnotations()
+   {
+     AlignFrame af = new FileLoader().LoadFileWaitTillLoaded(
+             "examples/uniref50.fa", DataSourceType.FILE);
+     AlignViewport av = af.getViewport();
+     AlignmentI al = av.getAlignment();
+     assertEquals(al.getWidth(), 157);
+     assertEquals(al.getHeight(), 15);
+     av.setWrapAlignment(true);
+     av.getRanges().setStartEndSeq(0, 14);
+     av.setFont(new Font("SansSerif", Font.PLAIN, 14), true);
+     int charHeight = av.getCharHeight();
+     int charWidth = av.getCharWidth();
+     assertEquals(charHeight, 17);
+     assertEquals(charWidth, 12);
+     SeqCanvas testee = af.alignPanel.getSeqPanel().seqCanvas;
+     /*
+      * first with scales above, left, right
+      */
+     av.setShowAnnotation(false);
+     av.setScaleAboveWrapped(true);
+     av.setScaleLeftWrapped(true);
+     av.setScaleRightWrapped(true);
 -    FontMetrics fm = SwingUtilities2.getFontMetrics(testee, av.getFont());
++    FontMetrics fm = testee.getFontMetrics(av.getFont());
+     int labelWidth = fm.stringWidth("000") + charWidth;
+     assertEquals(labelWidth, 39); // 3 x 9 + charWidth
+     /*
+      * width 400 pixels leaves (400 - 2*labelWidth) for residue columns
+      * take the whole multiple of character widths
+      */
+     int canvasWidth = 400;
+     int canvasHeight = 300;
+     int residueColumns = (canvasWidth - 2 * labelWidth) / charWidth;
+     int wrappedWidth = testee.calculateWrappedGeometry(canvasWidth, canvasHeight);
+     assertEquals(wrappedWidth, residueColumns);
+     assertEquals(PA.getValue(testee, "labelWidthWest"), labelWidth);
+     assertEquals(PA.getValue(testee, "labelWidthEast"), labelWidth);
+     assertEquals(PA.getValue(testee, "wrappedSpaceAboveAlignment"),
+             2 * charHeight);
+     int repeatingHeight = (int) PA.getValue(testee, "wrappedRepeatHeightPx");
+     assertEquals(repeatingHeight, charHeight * (2 + al.getHeight()));
+     assertEquals(PA.getValue(testee, "wrappedVisibleWidths"), 1);
+     /*
+      * repeat height is 17 * (2 + 15) = 289
+      * make canvas height 2 * 289 + 3 * charHeight so just enough to
+      * draw 2 widths and the first sequence of a third
+      */
+     canvasHeight = charHeight * (17 * 2 + 3);
+     testee.calculateWrappedGeometry(canvasWidth, canvasHeight);
+     assertEquals(PA.getValue(testee, "wrappedVisibleWidths"), 3);
+     /*
+      * reduce canvas height by 1 pixel - should not be enough height
+      * to draw 3 widths
+      */
+     canvasHeight -= 1;
+     testee.calculateWrappedGeometry(canvasWidth, canvasHeight);
+     assertEquals(PA.getValue(testee, "wrappedVisibleWidths"), 2);
+     /*
+      * turn off scale above - can now fit in 2 and a bit widths
+      */
+     av.setScaleAboveWrapped(false);
+     testee.calculateWrappedGeometry(canvasWidth, canvasHeight);
+     assertEquals(PA.getValue(testee, "wrappedVisibleWidths"), 3);
+     /*
+      * reduce height to enough for 2 widths and not quite a third
+      * i.e. two repeating heights + spacer + sequence - 1 pixel
+      */
+     canvasHeight = charHeight * (16 * 2 + 2) - 1;
+     testee.calculateWrappedGeometry(canvasWidth, canvasHeight);
+     assertEquals(PA.getValue(testee, "wrappedVisibleWidths"), 2);
+     /*
+      * make canvas width enough for scales and 20 residues
+      */
+     canvasWidth = 2 * labelWidth + 20 * charWidth;
+     wrappedWidth = testee.calculateWrappedGeometry(canvasWidth,
+             canvasHeight);
+     assertEquals(wrappedWidth, 20);
+     /*
+      * reduce width by 1 pixel - rounds down to 19 residues
+      */
+     canvasWidth -= 1;
+     wrappedWidth = testee.calculateWrappedGeometry(canvasWidth,
+             canvasHeight);
+     assertEquals(wrappedWidth, 19);
+     /*
+      * turn off West scale - adds labelWidth (39) to available for residues
+      * which with the 11 remainder makes 50 which is 4 more charWidths rem 2
+      */
+     av.setScaleLeftWrapped(false);
+     wrappedWidth = testee.calculateWrappedGeometry(canvasWidth,
+             canvasHeight);
+     assertEquals(wrappedWidth, 23);
+     /*
+      * add 10 pixels to width to fit in another whole residue column
+      */
+     canvasWidth += 9;
+     wrappedWidth = testee.calculateWrappedGeometry(canvasWidth,
+             canvasHeight);
+     assertEquals(wrappedWidth, 23);
+     canvasWidth += 1;
+     wrappedWidth = testee.calculateWrappedGeometry(canvasWidth,
+             canvasHeight);
+     assertEquals(wrappedWidth, 24);
+     /*
+      * turn off East scale to gain 39 more pixels (3 columns remainder 3)
+      */
+     av.setScaleRightWrapped(false);
+     wrappedWidth = testee.calculateWrappedGeometry(canvasWidth,
+             canvasHeight);
+     assertEquals(wrappedWidth, 27);
+     /*
+      * add 9 pixels to width to gain a residue column
+      */
+     canvasWidth += 8;
+     wrappedWidth = testee.calculateWrappedGeometry(canvasWidth,
+             canvasHeight);
+     assertEquals(wrappedWidth, 27);
+     canvasWidth += 1;
+     wrappedWidth = testee.calculateWrappedGeometry(canvasWidth,
+             canvasHeight);
+     assertEquals(wrappedWidth, 28);
+     /*
+      * now West but not East scale - lose 39 pixels or 4 columns
+      */
+     av.setScaleLeftWrapped(true);
+     wrappedWidth = testee.calculateWrappedGeometry(canvasWidth,
+             canvasHeight);
+     assertEquals(wrappedWidth, 24);
+     /*
+      * adding 3 pixels to width regains one column
+      */
+     canvasWidth += 2;
+     wrappedWidth = testee.calculateWrappedGeometry(canvasWidth,
+             canvasHeight);
+     assertEquals(wrappedWidth, 24);
+     canvasWidth += 1;
+     wrappedWidth = testee.calculateWrappedGeometry(canvasWidth,
+             canvasHeight);
+     assertEquals(wrappedWidth, 25);
+     /*
+      * turn off scales left and right, make width exactly 157 columns
+      */
+     av.setScaleLeftWrapped(false);
+     canvasWidth = al.getWidth() * charWidth;
+     testee.calculateWrappedGeometry(canvasWidth, canvasHeight);
+     assertEquals(PA.getValue(testee, "wrappedVisibleWidths"), 1);
+   }
+   /**
+    * Test the method that computes wrapped width in residues, height of wrapped
+    * widths in pixels, and the number of widths visible
+    */
+   @Test(groups = "Functional")
+   public void testCalculateWrappedGeometry_withAnnotations()
+   {
+     AlignFrame af = new FileLoader().LoadFileWaitTillLoaded(
+             "examples/uniref50.fa", DataSourceType.FILE);
+     AlignViewport av = af.getViewport();
+     AlignmentI al = av.getAlignment();
+     assertEquals(al.getWidth(), 157);
+     assertEquals(al.getHeight(), 15);
+   
+     av.setWrapAlignment(true);
+     av.getRanges().setStartEndSeq(0, 14);
+     av.setFont(new Font("SansSerif", Font.PLAIN, 14), true);
+     int charHeight = av.getCharHeight();
+     int charWidth = av.getCharWidth();
+     assertEquals(charHeight, 17);
+     assertEquals(charWidth, 12);
+   
+     SeqCanvas testee = af.alignPanel.getSeqPanel().seqCanvas;
+   
+     /*
+      * first with scales above, left, right
+      */
+     av.setShowAnnotation(true);
+     av.setScaleAboveWrapped(true);
+     av.setScaleLeftWrapped(true);
+     av.setScaleRightWrapped(true);
 -    FontMetrics fm = SwingUtilities2.getFontMetrics(testee, av.getFont());
++    FontMetrics fm = testee.getFontMetrics(av.getFont());
+     int labelWidth = fm.stringWidth("000") + charWidth;
+     assertEquals(labelWidth, 39); // 3 x 9 + charWidth
+     int annotationHeight = testee.getAnnotationHeight();
+     /*
+      * width 400 pixels leaves (400 - 2*labelWidth) for residue columns
+      * take the whole multiple of character widths
+      */
+     int canvasWidth = 400;
+     int canvasHeight = 300;
+     int residueColumns = (canvasWidth - 2 * labelWidth) / charWidth;
+     int wrappedWidth = testee.calculateWrappedGeometry(canvasWidth, canvasHeight);
+     assertEquals(wrappedWidth, residueColumns);
+     assertEquals(PA.getValue(testee, "labelWidthWest"), labelWidth);
+     assertEquals(PA.getValue(testee, "labelWidthEast"), labelWidth);
+     assertEquals(PA.getValue(testee, "wrappedSpaceAboveAlignment"),
+             2 * charHeight);
+     int repeatingHeight = (int) PA.getValue(testee, "wrappedRepeatHeightPx");
+     assertEquals(repeatingHeight, charHeight * (2 + al.getHeight())
+             + annotationHeight);
+     assertEquals(PA.getValue(testee, "wrappedVisibleWidths"), 1);
+   
+     /*
+      * repeat height is 17 * (2 + 15) = 289 + annotationHeight = 507
+      * make canvas height 2 * 289 + 3 * charHeight so just enough to
+      * draw 2 widths and the first sequence of a third
+      */
+     canvasHeight = charHeight * (17 * 2 + 3) + 2 * annotationHeight;
+     testee.calculateWrappedGeometry(canvasWidth, canvasHeight);
+     assertEquals(PA.getValue(testee, "wrappedVisibleWidths"), 3);
+   
+     /*
+      * reduce canvas height by 1 pixel - should not be enough height
+      * to draw 3 widths
+      */
+     canvasHeight -= 1;
+     testee.calculateWrappedGeometry(canvasWidth, canvasHeight);
+     assertEquals(PA.getValue(testee, "wrappedVisibleWidths"), 2);
+   
+     /*
+      * turn off scale above - can now fit in 2 and a bit widths
+      */
+     av.setScaleAboveWrapped(false);
+     testee.calculateWrappedGeometry(canvasWidth, canvasHeight);
+     assertEquals(PA.getValue(testee, "wrappedVisibleWidths"), 3);
+   
+     /*
+      * reduce height to enough for 2 widths and not quite a third
+      * i.e. two repeating heights + spacer + sequence - 1 pixel
+      */
+     canvasHeight = charHeight * (16 * 2 + 2) + 2 * annotationHeight - 1;
+     testee.calculateWrappedGeometry(canvasWidth, canvasHeight);
+     assertEquals(PA.getValue(testee, "wrappedVisibleWidths"), 2);
+     /*
+      * add 1 pixel to height - should now get 3 widths drawn
+      */
+     canvasHeight += 1;
+     testee.calculateWrappedGeometry(canvasWidth, canvasHeight);
+     assertEquals(PA.getValue(testee, "wrappedVisibleWidths"), 3);
+   }
+ }