Closed pjvandehaar closed 4 years ago
Interesting! I'd be very interested in using GeneLocator in lz-hosted as well. Would you mind if I sent a pull request to that repo with some suggested edits (likely tomorrowish)? Mostly cleanup + additional unit tests.
That would help us standardize how the library is called before it starts to be used in multiple places.
I've sent a pull request for the gene locator changes, here: https://github.com/pjvandehaar/genelocator/pull/1/
Once we sort out the disposition of that PR, we can update this one as needed and merge too.
To see behavior with multiple overlapping genes, try http://0.0.0.0:5000/variant/19_49928702/ . For the busiest location in the genome, try http://0.0.0.0:5000/variant/5_141489121/ .
This seems useful. From a styling standpoint, could we do something to collapse/relocate the list of overlapping genes? A long list forces the main page content down almost off the first screen:
Overall this is a style improvement; thanks!
Below is a suggested patch that does 3 things. I'm not sure about the resulting display, so thought I'd run it by you before merging. (I simplified the code, now we can add pizzazz)
I also autoformatted the HTML a bit (which explains the "whitespace changed" lines in the diff).
diff --git a/pheget/templates/phewas.html b/pheget/templates/phewas.html
index dee0d23..acf9c9c 100644
--- a/pheget/templates/phewas.html
+++ b/pheget/templates/phewas.html
@@ -16,6 +16,10 @@
.padtop {
padding-top: 20px;
}
+
+ .text-with-definition {
+ text-decoration: dotted underline;
+ }
</style>
<body>
@@ -45,25 +49,15 @@
<h1>chr{{ chrom }}:{{ "{:,}".format(pos) }} for all tissues and genes</h1>
<div>
- {% if nearest_genes|length == 1 %}
- {% with gene = nearest_genes[0] %}
- <p>{{ "Overlapping" if gene.start <= pos <= gene.end else "Nearest" }} gene:
- <i>{{ gene.symbol }}</i> (<i>{{ gene.ensg }}</i>)
- chr{{ gene.chrom }}:{{ "{:,}".format(gene.start) }}-{{ "{:,}".format(gene.end) }}</p>
- {% endwith %}
- {% elif nearest_genes|length <= 5 %}
- <p style="margin-bottom:0">Overlapping genes:</p>
- <ul>
- {% for gene in nearest_genes %}
- <li>chr{{ gene.chrom }}:{{ "{:,}".format(gene.start) }}-{{ "{:,}".format(gene.end) }} <i>{{ gene.symbol }}</i>
- (<i>{{ gene.ensg }}</i>)
- </li>
- {% endfor %}
- </ul>
- {% else %}
- <p>Overlapping genes: {% for gene in nearest_genes %}<i>{{ gene.symbol }}</i>{% if not loop.last %},
- {% endif %}{% endfor %}</p>
- {% endif %}
+ <p>
+ {{ "Overlapping" if is_inside_gene else "Nearest" }} gene(s):
+ {% for gene in nearest_genes %}
+ <span class="text-with-definition" title="{{ gene.ensg }}">{{ gene.symbol }}</span>
+ {%- if not loop.last -%},{% endif %}
+ {% else %}
+ (no genes found)
+ {% endfor %}
+ </p>
</div>
<div class="btn-group btn-group-toggle" data-toggle="buttons">
diff --git a/pheget/views/variant.py b/pheget/views/variant.py
index 1cff2e9..be2a1e5 100644
--- a/pheget/views/variant.py
+++ b/pheget/views/variant.py
@@ -3,6 +3,7 @@ Variant View Page.
"""
from flask import render_template
+from genelocator import exception as gene_exc
from genelocator import get_genelocator
import pheget
@@ -16,5 +17,16 @@ def variant_view(chrom_pos):
# TODO: Allow query params to be passed from the base page to the api endpoint, so user can direct link to a
# custom view
chrom, pos = parse_position(chrom_pos)
- nearest_genes = gl.at(chrom, pos)
- return render_template('phewas.html', chrom=chrom, pos=pos, nearest_genes=nearest_genes)
+
+ try:
+ nearest_genes = gl.at(chrom, pos)
+ except (gene_exc.NoResultsFoundException, gene_exc.BadCoordinateException):
+ nearest_genes = []
+
+ # Are the "nearest genes" nearby, or is the variant actually inside the gene?
+ # These rules are based on the defined behavior of the genes locator
+ is_inside_gene = (len(nearest_genes) > 1 or
+ (len(nearest_genes) == 1 and nearest_genes[0]['start'] <= pos <= nearest_genes[0]['end']))
+
+ return render_template('phewas.html',
+ chrom=chrom, pos=pos, nearest_genes=nearest_genes, is_inside_gene=is_inside_gene)
That change looks great. Feel free to push it to this branch. I had never cared about codestyle enough to use {%-
but I'm glad that we now have a use.
That change looks great. Feel free to push it to this branch. I had never cared about codestyle enough to use
{%-
but I'm glad that we now have a use.
Rather than purely stylistic, it's actually a whitespace control feature: it allows me to display gene, gene2
instead of gene , gene2
. Not well known but pretty handy.
Merging our combined changes. Thanks again!
Ticket: #18