Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alpha #73

Merged
merged 30 commits into from Jul 11, 2015
Merged

Alpha #73

merged 30 commits into from Jul 11, 2015

Conversation

IsmailM
Copy link
Member

@IsmailM IsmailM commented Jun 10, 2015

  • Instead of producing 5 JSONs per sequences, GV now would only produce a single results JSON that contains all the results for that sequence.
  • This removes a lot of excess info in the HTML (significantly decreasing the size of the output HTML file)
  • Also I have removed the eval() in the function running 'show all plots' (so this could potentially fix that crashing problem (Browser Crashes if 'Show all Plots' button is pressed, when there are a large number of queries #55) - but I haven't tested it yet).

Other Points

  • Split the Validation Class into Validations and Validate Classes (so a new Validate Class is initiated for each sequence)...
  • Remove Some Dead Code
  • Fix most of the JsHINT points with plot.js
  • Only create the pagination (added last PR) when there are more than one page
  • Add space between the table header and the tooltip icon.

@IsmailM
Copy link
Member Author

IsmailM commented Jun 11, 2015

A typical row and child row produced in this alpha:

<tr data-target="toggle3" data-jsonFile="files/json/5000.fa_3.json">
    <td>3</td>
    <td data-score="50">
        <div class="ratings">
            <div class="empty-stars"></div>
            <div class="full-stars" style="width:42.5%;"></div>
        </div>
    </td>
    <td title="Definition" class="seq_definition">AP201406-maker-Si_gnH.scaffold00721-snap-gene-0.12-mRNA-1 protein AED:0.35 eAED:0.37 QI:0|0|0|1|0|0|2|0|126</td>
    <td title="No. Hits">6</td>
    <td title="Length Cluster" class="danger my-btn-danger" onclick="addData(this, 'LengthCluster');">126&nbsp;[1690,&nbsp;1707]</td>
    <td title="Length Rank" class="danger my-btn-danger" onclick="addData(this, 'LengthRank');">0%&nbsp;(too&nbsp;short)</td>
    <td title="Gene Merge" class="success my-btn-success" onclick="addData(this, 'Gene_Merge');">1.4</td>
    <td title="Duplication" class="success my-btn-success" onclick="addData(this, 'Duplication');">1.0</td>
    <td title="Missing/Extra sequences" class="warning">Not enough evidence</td>
    <td>
        <button title="Show plots" name="plot_btn" class="plot_btn btn btn-default" onclick="addData(this, 'all');"><i class="fa fa-bar-chart-o"></i></button>
    </td>
</tr>
<tr class="tablesorter-childRow" name="plot_row" style="display:none;">
    <td colspan="12" id="toggle3row">
        <div id="toggle3" class="expanded-child"></div>
    </td>
</tr>

@IsmailM
Copy link
Member Author

IsmailM commented Jun 11, 2015

Secondly, with regards to the 'Show all plots' button - it no longer crashes the browser if you try to open show a large number of plots...

For this test, I temporarily (manually) disabled the safeguard that disables the 'Show all plots' button if there are more than 30 rows...

With a large dataset (5000 rows), it managed to open over 1300 rows before I paused the script (via firebug)... However, it took some time (i.e. quite a few minutes) to open these rows (it accessed over 1000 JSONs to draw all those plots)....

@IsmailM
Copy link
Member Author

IsmailM commented Jun 18, 2015

Further Updates:

  • Move td onclick to a event handler
  • decrease the max of rows in a single HTML file to 2500 (as @yannickwurm suggested)
  • Add info on how to filter results to the README (i.e. how to filter JSON)
  • Make it possible to create GV HTML results from a (filtered) JSON...
 $ genevalidator -j INPUT_JSON_FILE

@yeban can you have a look at this - I think it is now ready to be merged...

@IsmailM
Copy link
Member Author

IsmailM commented Jun 18, 2015

A typical Row

<tr data-target="toggle4" data-jsonFile="files/json/input.fa_4.json">
  <td title="idx">4</td>
  <td data-score="100"><div class="ratings"><div class="empty-stars"></div><div class="full-stars" style="width:85.0%;"></div></div></td>
  <td title="Definition">GB10058-PA</td>
  <td title="No. Hits">114</td>
  <td title="Length Cluster" class="danger">1052&nbsp;[1106,&nbsp;1567]</td>
  <td title="Length Rank" class="success">40%</td>
  <td title="Gene Merge" class="success">-0.3</td>
  <td title="Duplication" class="success">1.0</td>
  <td title="Missing/Extra Sequences" class="success">83%&nbsp;conserved; 2%&nbsp;extra; 19%&nbsp;missing.</td>
  <td><button title="Show plots" class="plot_btn btn btn-default"><i class="fa fa-bar-chart-o"></i></button></td>
</tr>

@yeban
Copy link
Contributor

yeban commented Jun 24, 2015

-j functionality is kick-ass. Codwise there's way too many changes to go through. I'm running "the 5000-sequences test". Will get back to you when it's done.

@yeban
Copy link
Contributor

yeban commented Jun 24, 2015

So this time I loaded the results from http://antgenomes.org/~priyam/5000.fa.html/results2.html (still hosted).

Loads for me in Chrome, Firefox and Safari. Fluid enough in all three browsers (scrolling, viewing plots per hit one by one - tried 30 hits). Each result.html is now ~2MB - much better. Sorting is gone - sucks. Load all charts - is enabled (bug?) - Chrome rendered responsively (could smoothly scroll through the page) till the page crashed, Firefox & Safari became unresponsive while trying to render till I force quit them (closing the tab wasn't working either). Right click on the page takes a lot of time.

Performance consideration aside, the way show_all_plots works is bad UX. It is not guaranteed that plot data will be loaded / rendered row by row. So graphs for the second hit may be rendered first and I start looking at it, and in the meantime graphs for the first hit get rendered and the part of the page I was viewing gets pushed out of the view - very annoying (typical of news websites).

I was wondering why not name the json files as xx_0001.json (zero padding the number) so it ls lists them in order.

}

$("td, .plot_btn").click(toggleRow);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This binds the same event handler 2500 times, once per row. Pretty wasteful, don't you think? It's preferred to use event delegation in such cases - http://learn.jquery.com/events/event-delegation/

@IsmailM
Copy link
Member Author

IsmailM commented Jul 11, 2015

I'm merging this now....

The current master is breaking due to breaking changes in statsample...

I'll work on event delegation that @yeban pointed out at a later date (i.e. after 20th) ...

For further changes, branch the master and submit a new PR...

IsmailM added a commit that referenced this pull request Jul 11, 2015
@IsmailM IsmailM merged commit 1714094 into master Jul 11, 2015
@IsmailM IsmailM deleted the alpha branch July 12, 2015 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants