-
-
Notifications
You must be signed in to change notification settings - Fork 371
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
Dashboard fixes #100
Dashboard fixes #100
Conversation
It would also be good if it wouldn't re-run the entire pipeline if only the transfer fn changes. That way people can quickly try out different transfer_fns. |
@brendancol, while this is all fresh in your mind, there are some other small dashboard-related fixes in the issues list that could be addressed before you move on to other things... |
@@ -28,7 +31,6 @@ | |||
from webargs import fields |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How should we handle the dashboard's dependence on webargs? Should we just state that in the README? I don't think we'd necessarily want to make webargs a dependency of datashader, at least not until conda supports optional dependencies.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's just an example, so my opinion is that the dependencies don't matter as long as they're listed explicitly somewhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do need to make it easy for people to run the example, though. After getting an error message about it, I briefly looked for webargs on conda, tried some bogus versions from non-main channels that didn't work, and eventually pip-installed webargs (which worked). Most people probably aren't that dedicated.
Is the area highlighted by the hover tool correct? It doesn't seem to be. E.g. there's a hotspot just to the left of the blue area: but if I hover directly over that, seemingly enclosing the hotspot in a blue box, the counts aren't particularly high: Yet if I move the mouse down to the cell below and to the left, I get high counts indicative of a hotspot: Does the displayed blue box need to be moved to accurately reflect the area in the hover information? |
The behavior in the corners with a large hover-box size also looks suspicious -- shouldn't the box be the same size throughout the array (with at most a pixel of rounding more or less), not cropped to a quarter the size in the corners? |
I guess this may be addressed by the proposed switch to averaging pixel values for reaggregation, but I can't quite make sense of the values for some combinations of Field and Aggregate. E.g.: What does counting the Fare field mean? Counting how many non-empty Fare values there are? If so, presumably it's not in |
Maybe report "Avg Fare ($) Count: xxxx", once it's averaging the values instead of re-aggregation? I.e., show the aggregate explicitly, not just the field? |
I added some commits for some useful bits, including out-of-core operation based on code from jcrist, but it's not quite ready to merge because the link to the census.castra file is blank (because I haven't yet heard back about options for hosting that file). It's also strange that castra has to be downloaded from a special channel; is there any way to get castra from a more public place (@jcrist?) |
…egend to left-hand controls menu
x_end=max_val, | ||
y_range=(0,18)) | ||
|
||
self.model.legend_vbox.children = [legend_fig] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once it settles down a bit, it would be good to move the legend support into a function or a class, with appropriate parameters, with an eye to eventually moving it out of the dashboard.py file and into a datashader library file.
This PR adds:
It's also close to adding support for colorization by census race categories, but I'm not sure how to add that in a general way. Below are some trivial diffs that are sufficient to get racial color categories shown, in a hardcoded way that removes support for non-categorical data. With this PR and those diffs applied, there will initially be an error on startup because the default Field is counts in census.yml, but if the Field is then changed to Race to match the new Count Categories aggregate declared below, it should work.
So, @brendancol, can you take it from here? Can you make dashboard.py support categorical information where appropriate? I've added the race colors to the census.yml file already, but it would take me a while to figure out how to look that information up when needed, based on the "cat_colors" pointer I added to census.yml (but which could be changed if needed). There will also of course need to be some logic to switch between tf.interpolate and tf.colorize.
If we want to avoid errors for nonsensical combinations, just as for the earliest client.py file from ages ago, we'll presumably need to start declaring datatypes for these objects so that the buttons generally work rather than generally fail...