Refactoring: correlate_geographies

[milafrerichs]

Hey,

I took some time to refactor the correlate_geographies. Hope that is ok with you. I think it's more readable and easier to break down. Hope that is ok with you :) Included tests too.

Regarding #13 how would you like to split it up?

[migurski]

Hey thanks Mila! Can you tell me a little more about the changes? I see a window.setTimeout() added with 4000 msec argument. Does that mean the correlation doesn’t happen for 4 seconds?

[milafrerichs]

Yes that was just put in there to discuss with you where do you want to put the timeouts. I would just start the correlation as the last call. Maybe with a few milliseconds delay.

The changes are all just refactorings. I'm using underscore.js to get rid of all the loops. And I added a few functions to the Tract "class".

[migurski]

Without loops, it might be more complicated to spread the process out over time. Does underscore have some concept of that? Can each() work in an interruptible way?

[milafrerichs]

No it does not. but how would you have done it anyway?

Would you've split up the work inside the response loop? So of every response?

The most expensive method is the turf calculations (https://github.com/codeforamerica/resident-web-use-research/blob/master/lib.js#L290 and https://github.com/codeforamerica/resident-web-use-research/blob/master/lib.js#L294). Nothing we can really do. As I understand it there are unique for every tract-response combination.

[milafrerichs]

There is a defer function in underscore

Defers invoking the function until the current call stack has cleared, similar to using setTimeout with a delay of 0. Useful for performing expensive computations or HTML rendering in chunks without blocking the UI thread from updating.

Or we could use some libraries like Q or when.js

[migurski]

Yeah, either a defer with underscore or a window.setTimeout with plain Javascript. In either case, I don’t think we can simply map over all the combinations. We will have to make a list of all n×m combinations, iterate over that list until some time limit is reached, then schedule the remainder of the list for later pickup.

[migurski]

Hey @milafrerichs, curious what you think about the defer/timeout thing above?

[milafrerichs]

@migurski yes I think it's a good thing. I changed the setTimeout to use defer I think we should just add a modal loading screen to make the user aware that some computation is needed. you included that in the #13 already. can work on that in a bit...

[milafrerichs]

Staging Preview

[migurski]

Seeing this: lib.js:506 Uncaught ReferenceError: L is not defined

[milafrerichs]

It's now working. For an explanation see https://github.com/codeforamerica/resident-web-use-research/pull/17#issuecomment-153281687

Staging Preview

[milafrerichs]

Merged the time based optimization from #23 Added more tests and merged master