microscopium / microscopium-ui

Microscopium web interface.
2 stars 4 forks source link

[WIP] Add filtering functionality and testing framework #40

Closed starcalibre closed 9 years ago

starcalibre commented 9 years ago

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..

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..

    .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. :)

jni commented 9 years ago

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 commented 9 years ago

Alrighty, some usage notes:

On the plus side:

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! =)

starcalibre commented 9 years ago

@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:

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 commented 9 years ago

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

starcalibre commented 9 years ago

@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 commented 9 years ago

@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 commented 9 years ago

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

starcalibre commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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

jni commented 9 years ago

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,

jni commented 9 years ago

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

starcalibre commented 9 years ago

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 https://github.com/microscopium/microscopium-ui/pull/40#issuecomment-90530883 .

jni commented 9 years ago

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! =)

jni commented 9 years ago

@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:

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 commented 9 years ago

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

jni commented 9 years ago

@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?

starcalibre commented 9 years ago

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 commented 9 years ago

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.