Skip to content

Conversation

@nielseneli
Copy link

This change is Reviewable

@jenwei
Copy link

jenwei commented Apr 4, 2016

@nielsenlouise + @apurvaraman - nice job on this and working with Bokeh - it's tough for sure.

In terms of your writeup, it was well-organized, clear, and thoughtful. Good point about being more cautious when scoping out a project that uses a library you're unfamiliar with. Also, good note about using pandas as recommended by Bokeh, though I'm sure if searching through dictionaries would've been that big of a bottleneck since you can do direct lookups in dictionaries and not necessarily have to "iterate" through the dictionary (though you're right - you would have to go through all the separate category dictionaries).

I'll add comments to your code after this.



if __name__ == '__main__':
thing = View()
Copy link

Choose a reason for hiding this comment

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

Overall - code looks good! It was nicely designed and well documented. For the most part, variable names were sensibly chosen.

You should include a .gitignore file in your repository to prevent *.pyc files from being pushed. For more info on .gitignore, see https://git-scm.com/docs/gitignore

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.

2 participants