Skip to content
This repository has been archived by the owner on Feb 8, 2019. It is now read-only.

[WIP] Add filtering functionality and testing framework #40

Merged
merged 25 commits into from
Apr 16, 2015

Conversation

starcalibre
Copy link
Member

  • The much anticipated WORKING filtering function! It works as I'd expect so far, but would appreciate it if you could mess around with it locally and try and break it :)
  • Uility functions such as regexFilter have been moved to their own JS file. Why? Because..
  • Tests!!!!!

I was looking at tooltips to check the filtering was working as expected.. I realised quickly this was stupid because the functions used to filter the data are easily testable. You know how in Python you simply import pytest, write your tests, run them and you're good to go? Well, it's pretty much the same in JS...AHAHAHAHAHA yeah right. It's a little more involved.. I'll try an explain how it works:

Testing in JS needs two components..

  • A testing framework -- I'm using Jasmime.
  • A test runner -- I'm using Karma
    (Other choices here include "Chai" and "Mocha".. I swear I'm not making this shit up. I've chosen Jasmine and Karma because I'm most familiar with them and they're currently the most popular. Though I'm sure 'Affogato' has just been released and is all the rage.. until next week 'Decaf Latte' comes out and is the testing suite du jour. :P :P :P).

So, how it works is.. you write your tests in Jasmine in a JS file, then this file gets sent to Karma. Karma usually runs in a browser (because you need something to run your JS), and it gives you the results of the tests in a HTML file.

However, I find this a little cumbersome.. and I want to add the tests to my Grunt pipeline, so I can see how my tests went along with my JSHint every time I change the code. That's where PhantomJS comes in. It's a 'headless' browser, which means it's a browser with no GUI that doesn't render the DOM. It just keeps the DOM in memory and is able to run JavaScript though, so Karma can use it to run the tests. This usually makes testing faster, because you don't have the overhead of the browser GUI and DOM rendering. The results of the tests can then be outputed to your terminal running Grunt.

The above wall of text suggests this is complicated to setup, but the grunt plugin grunt-contrib-karma does most of this for you.

Two things outstanding..

  • Currently the filter is removing points, rather than changing the opacity. I didn't get much further with this since tinkering it with it at Zouki's the other afternoon. I'll work on this, but thought I should send this PR sooner rather than later.
  • This particular chunk of code:
    .append('circle')
        .attr('cx', function(d) {
            return self.xScale(d.pca[0]);
        })
        .attr('cy', function(d) {
            return self.yScale(d.pca[1]);
        })
        .attr('r', 5)
        .classed('scatterpt', true)
        .classed('activept', false)
        .classed('neighbourpt', false)
        .attr('id', function(d) {
            return d._id;
        })
        .on('mouseover', self.tip.show)
        .on('mouseout', self.tip.hide)
        .on('click', function(d, i) {
            var selection = d3.select(this);
            if(!selection.classed('activept')) {
                self.updatePoint(selection, d, i);
            }
        });

appears about 4830439593 times so I'm going to put it into its own function. It's a bunch of chained methods though, so I just need to work out how to do with this without making D3 crap the bed.

FInally.. If you run this branch locally, remember to npm install so you download everything you need to run the tests. :)

@starcalibre starcalibre changed the title Add filtering functionality and testing framework [WIP] Add filtering functionality and testing framework Mar 26, 2015
@jni
Copy link
Contributor

jni commented Mar 26, 2015

Coolness! This was a lot to process but you made it very entertaining LOL. I'll try to review this tomorrow, still struggling with some microscopium changes (the CLI needed some TLC... =P)

@jni
Copy link
Contributor

jni commented Mar 30, 2015

Alrighty, some usage notes:

  • fix alpha instead of disappearing? Should that be flagged in this PR?
  • On the MYORES screen, I click on Filter > type slc, a whole bunch of genes come up.
    • I have to click on the gene box before pressing enter will do anything. (After the first click I can just press Enter repeatedly.) Can this be fixed? (I'd just like to be able to type, then Enter, no mousing.)
    • When I press "Reset", the list of selected genes doesn't reset until I try to type in the selection box again and then erase.
    • When the filter is active, selecting a gene doesn't change the glyph colours to red/orange, as would be expected. I think that should still be maintained, and we should still show the filtered out neighbours, but with the image greyed out (or with lower alpha), and with smaller glyphs. The nice thing is that the glyph properties can be independent: filtering reduces size and alpha, selecting changes colour.
    • The above issue (glyph colour doesn't change when filtering is active) is irreversible: even once the filter is reset, clicking on the samples still doesn't change the glyph colour, for the selection or for the neighbours.
    • double clicking a gene in the gene list should have the same effect as pressing Enter (add that gene to the filter).
  • nitpicky: clicking the "row" drawer closes the gene drawer very abruptly. It feels broken. Can the gene drawer be made to close gradually (the same way it opens)?
  • When filtering on columns, I have to click "reset" twice to make the "filter" icon return to gray.

On the plus side:

  • Filtering works, that's awesome! =)
  • combining filters (e.g. genes and columns) also works, which is also awesome. =)
  • testing! Brilliant. (As you yourself noted with a quintuple exclamation mark. =D) Also, gotta love seeing the little unicode tick marks on the terminal. =D
  • PhantomJS: Brilliant! =D That seems like exactly what you would want for a testing framework! Very happy that this exists.

You seem to know what to do next, anyway. Regarding your massively chained methods, it is indeed unacceptable that they would appear more than once! Why would chaining methods be a problem when defining another method? e.g. in Python, if I want to add a fit_predict method to a scikit-learn classifier, I can just go:

def fit_with_training_error(self, X, y, loss=absolute_deviation):
    predictions = self.fit(X, y).predict_proba(X)
    return loss(predictions, y)

Anyway, I won't comment on the code while you're still working on it. I hope at least some issues above were undiscovered! =)

Future me will thank me if I need to add other checkboxes to any other part of the UI.
* The sample indices get shuffled around during the d3 enter/update/exit/pattern, so use the sample _id's to lookup data instead
@starcalibre
Copy link
Member Author

@jni The commits above fixed the issues that I have checked off! With respect to the outstanding points..

  1. Yeah.. absolutely. I'm working on making it so that the the overlapping opaque SVG elements don't 'stack' and increase the opacity. This should be possible using the SVG filter. It's a 'nice to have' feature for the moment though, if it gets too complex I'll set it aside for the moment.
  2. The points should still retain their changes of colour if they're just having their alpha tweaked. Shouldn't be too hard to change the class of filtered points in the image display. I'll just need to define a class that changes the opacity of the div that the images sits in, and apply it conditionally.
  3. That feels broken because I think it is! I think it's a problem with how it's defined in the HTML. Looking into it now!

New issue that's become apparent:

  • Updating the filter resets the styling of the selected/neighbour points. Need to retain the styling of the points despite the status of the filter.

I like the checkboxes in the terminal too! I also think Jasmine's way of using strings to describe how the tests should behave is great.

@jni
Copy link
Contributor

jni commented Apr 2, 2015

Really awesome! I didn't even think that you could prevent the alpha stacking, but that would be absolutely brilliant! =D

Other than that, excellent progress, I'm excited! =D

* Add opacity as parameter to set in constructor
* extend d3.selection object with moveToBack method
* change opacity of plot points when they're not in the filter
@starcalibre
Copy link
Member Author

@jni -- Just pinging you to let you know there's been a few updates. Check them out if you'd like. Currently all that's outstanding is getting the SVG opacities to stop stacking. It's weird, I can get it to work when I place all the points in a <g> container and apply the filter to that, but not when I apply it to the points directly. Yay SVG :P

@jni
Copy link
Contributor

jni commented Apr 7, 2015

@starcalibre fantastic! I'd seen some of these earlier today but I'm still fixing the mess I made when I assumed there would be three channels in the input stream instead of 1-3. =P I'll look this over very soon!

@jni
Copy link
Contributor

jni commented Apr 7, 2015

btw is it very dramatic to add some <g> tags all over the place? =P

@starcalibre
Copy link
Member Author

I feel your pain :P This was the exact problem I had when I wrote the original batch stitching function.. it was a classic "oh this is easy....." 4 hours later "throws computer out window* moment. Is there something about the way I went about it that's incompatible in your new code?

Not really, I'm gonna try a solution where I move SVGs between two <g> groups.. one for points being filtered, and one for those that aren't.

<g id="apply-filter" filter="url(#constantOpacity)">
  // points that are being filtered out
</g>
<g id="don't-apply-filter">
  // points not being filtered out
</g>

This is also nice because the points not being filtered will be rendered after the points being filtered, so they'll always appear on top.

@jni
Copy link
Contributor

jni commented Apr 7, 2015

is there something [...] incompatible in your new code?

No, not really, I just need to move stuff around, ensure the API is nice (ie specify blank channels with None, otherwise just stitch however many channels are present... I might even add in a squeeze so that it works fine with montaging single channels), and make sure the right number of images are plucked for the stream at any given time...

And make sure I don't break your existing stuff! =P

@jni
Copy link
Contributor

jni commented Apr 7, 2015

@starcalibre are you still working on this? Currently, performance is excruciating. Set filter, spinning beach ball of death... eventually the filter works. In addition, filtering for PLK in the 10A screen results in everything disappearing...

@starcalibre
Copy link
Member Author

Are any errors reported to the console? It's running fine on my end. However I've only tested in Chrome so far, I'm away from my machine running Firefox and IE at the moment.

@jni
Copy link
Contributor

jni commented Apr 7, 2015

Console log looks clean. Let me try it in Chrome, see if I get better results. (Was using Safari/OSX.)

@jni
Copy link
Contributor

jni commented Apr 7, 2015

Well the good news is that I was being an idiot and I hadn't correctly pulled in git. =P

The bad news is that now I don't even get to the testing... My spinner just keeps on spinning, even after the screen info comes up. At that point I can do nothing other than reload the page. This is on Safari, and emptying the cache didn't help.

This is the only error-y output in the console:

Running "jshint:all" (jshint) task
Linting public/js/Filter.js ...ERROR
[L177:C16] W033: Missing semicolon.
        }, 400) 
Linting public/js/NeighbourPlot.js ...ERROR
[L177:C48] W033: Missing semicolon.
    self.updatePoint(first, this.sampleData[0]) 
[L246:C43] W033: Missing semicolon.
            return !_.contains(ids, d._id) 
[L251:C26] W033: Missing semicolon.
            .moveToBack() 
Linting public/js/script.js ...ERROR
[L31:C10] W083: Don't make functions within a loop.
        });

In Chrome,

  • filtering doesn't appear to work (everything goes transparent, not just filtered-out stuff), and
    screen shot 2015-04-07 at 10 11 07 pm
  • pressing enter when searching in the gene box gives me in an "no data received" blank page.

@jni
Copy link
Contributor

jni commented Apr 7, 2015

@starcalibre are you able to meet up around VLSCI tomorrow or Thurs? We can have a mini-sprint perhaps? =)

@starcalibre
Copy link
Member Author

Is there anything reported to the browsers console? In Safari or Chrome?

I'll be in tomorrow, I can come in after midday?
On 07/04/2015 10:15 PM, "Juan Nunez-Iglesias" notifications@github.com
wrote:

Well the good news is that I was being an idiot and I hadn't correctly
pulled in git. =P

The bad news is that now I don't even get to the testing... My spinner
just keeps on spinning, even after the screen info comes up. At that point
I can do nothing other than reload the page. This is on Safari, and
emptying the cache didn't help.

This is the only error-y output in the console:

Running "jshint:all" (jshint) task
Linting public/js/Filter.js ...ERROR
[L177:C16] W033: Missing semicolon.
}, 400)
Linting public/js/NeighbourPlot.js ...ERROR
[L177:C48] W033: Missing semicolon.
self.updatePoint(first, this.sampleData[0])
[L246:C43] W033: Missing semicolon.
return !_.contains(ids, d._id)
[L251:C26] W033: Missing semicolon.
.moveToBack()
Linting public/js/script.js ...ERROR
[L31:C10] W083: Don't make functions within a loop.
});

In Chrome,

filtering doesn't appear to work (everything goes transparent, not
just filtered-out stuff), and
[image: screen shot 2015-04-07 at 10 11 07 pm]
https://cloud.githubusercontent.com/assets/492549/7022839/7aa027c2-dd73-11e4-9fc0-3a30ee5ac26c.png

pressing enter when searching in the gene box gives me in an "no data
received" blank page.


Reply to this email directly or view it on GitHub
#40 (comment)
.

@jni
Copy link
Contributor

jni commented Apr 7, 2015

Woot, I'll see you then! Maybe we can go to the University club. =)

Here's some ominous-looking things when I press Enter in the gene search box:

http://localhost:8080/?
plate%5B%5Bobject+Object%5D%5D=true&plate%5B%5Bobjec…rue&col%5B21%5D=true&col%5B22%5D=true&col%5B23%5D=true&col%5B24%5D=true#:1 GET http://localhost:8080/?plate%5B%5Bobject+Object%5D%5D=true&plate%5B%5Bobjec…D=true&col%5B21%5D=true&col%5B22%5D=true&col%5B23%5D=true&col%5B24%5D=true
net::ERR_EMPTY_RESPONSE

Let's try to figure it out in the morning. Goodnight! =)

* Prevent default browser behavour on submit
* Hitting enter on textbox updates gene filter
* Cache jQuery selectors and use selector chaining where possible
* The use of the SVG opacity filter has been removed. This was proving to be really computationally intensive, particularly in Firefox and Safari. The opacity is now set at a low value (0.25), and this can be adjusted in the NeighbourPlot constructor. The opacity value stacks, but points not being filtered are moved to the front so they're visible.
* The opacity set each time the plot updates by reducing the opacity of ALL points in the plot, then increasing the opacity of all points that are currently being filtered. This is done by iterating the points. D3's update-select-exit pattern does not seem particularly well suited to this task.
* Where possible, I am caching the selectors in jQuery.
* Finally, a few selectors and HTML tags have been updated to ensure the Filter loads correctly when the screen is changed.
@jni
Copy link
Contributor

jni commented Apr 15, 2015

@starcalibre Are you done with this? It's basically ready, imho! There's a couple of minor bugs, and it's a bit sluggish, but I'm happy to leave these things to later developments. What do you think?

The bugs I'm talking about:

  • If I combine a row filter and a gene filter, when I press "Reset", only the gene filter is reset, not the row filter.
  • Even after resetting the gene filter and manually re-adding all the rows, some glyphs were still not returning to normal (screenshot below). The filter icon had correctly returned to gray, indicating that all filtering was supposedly turned off.

Feel free to clear these tickboxes either by piling on commits here or by creating issues. =)

screen shot 2015-04-15 at 6 32 43 pm

@starcalibre
Copy link
Member Author

@jni - Done, with a little bonus optimization thrown in

@jni
Copy link
Contributor

jni commented Apr 16, 2015

@starcalibre Fan-friggin-tastic. Merging, finally. =D

btw, once filters are active, everything slows way down... Is that just because of the transparency, or is there some additional continuing processing going on?

jni added a commit that referenced this pull request Apr 16, 2015
[WIP] Add filtering functionality and testing framework
@jni jni merged commit aad4056 into microscopium:master Apr 16, 2015
@starcalibre
Copy link
Member Author

Right now I suspect it's the transparency -- the performance was fine until that was introduced. I'm playing around the profiling tools in Firefox now, I'll see if that can reveal anything.

@jni
Copy link
Contributor

jni commented Apr 17, 2015

A quick thing to try might be to replace transparency with just a very light blue colour, see if that improves things.

I'm raising an issue to continue this discussion. See #42.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants