alan-turing-institute / TuringDataStories

TuringDataStories: An open community creating “Data Stories”: A mix of open data, code, narrative 💬, visuals 📊📈 and knowledge 🧠 to help understand the world around us.
Other
39 stars 12 forks source link

[Turing Data Story] Election model #111

Closed edaub closed 3 years ago

edaub commented 3 years ago

Summary

New story using a Bayesian Hierarchical model to examine the mail votes for several battleground states in the 2020 US Presidential Election. Implements the model in a markdown file/notebook with additional documentation/requirements specificiations. Fixes #108.

List of changes proposed in this PR (pull-request)

What should a reviewer concentrate their feedback on?

Acknowledging contributors

review-notebook-app[bot] commented 3 years ago

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

crangelsmith commented 3 years ago

Hi @edaub, this is great thanks!! We will start the review process, @samvanstroud and @kevinxufs are going to be the reviewers. A couple of comments:

  1. We might need to do some formatting changes to the notebook to make it compatible with the fast pages interface, I hope this is Ok.
  2. As we are very close to the Christmas break we might not finish the review until the new year, but we hope to have this published sometime in January.

I'll open the review issue now.

crangelsmith commented 3 years ago

@all-contributors please add @edaub for blog

crangelsmith commented 3 years ago

@all-contributors please add @edaub for code

allcontributors[bot] commented 3 years ago

@crangelsmith

I've put up a pull request to add @edaub! :tada:

crangelsmith commented 3 years ago

@all-contributors please add @edaub for ideas

allcontributors[bot] commented 3 years ago

@crangelsmith

I've updated the pull request to add @edaub! :tada:

allcontributors[bot] commented 3 years ago

@crangelsmith

I've updated the pull request to add @edaub! :tada:

crangelsmith commented 3 years ago

@all-contributors please add @edaub for content

allcontributors[bot] commented 3 years ago

@crangelsmith

I've updated the pull request to add @edaub! :tada:

crangelsmith commented 3 years ago

@all-contributors please add @martintoreilly for ideas

allcontributors[bot] commented 3 years ago

@crangelsmith

I've put up a pull request to add @martintoreilly! :tada:

edaub commented 3 years ago

No problem, @crangelsmith! Thanks for the help with this and whenever they have a chance to review I am sure it will be helpful. I'm happy to help with formatting with some further changes to the documents if that would be useful.

samvanstroud commented 3 years ago

Hi @edaub, thanks for submitting this! @kevinxufs and I are getting started with our reviews, we hope to have them done in a week's time. The reviews will happen at https://github.com/alan-turing-institute/TuringDataStories/issues/113.

edaub commented 3 years ago

I've pushed my final updates to the markdown file, and I think I am happy with this version. In addition to the revisions suggested and discussed in #113, I ended up refactoring some of the code for modularity/clarity, including the animations.

I still need to convert to a notebook and run the expensive simulations. @crangelsmith Do we know the best way to handle the animations for the final version? I'm not sure how the deployment occurs -- will the saved notebook be published unexecuted, shown directly as I save it, or automatically run again when it is published? Similarly, if we want to publish an HTML version that will show the animations but not let the other code be executed, do we know how this might work? Happy to help out in managing this however I can -- I certainly don't want to make more effort for you!

crangelsmith commented 3 years ago

Hi @edaub , thanks for this, we will be publishing the story imminently :D

About the format for the final version, I think @samvanstroud did a check before and the executed notebook renders nicely in fast pages, so we might not have to do much on that side. He can confirm this.

Also I wanted to tell you that I plan to use this story for a hand-on session i'll be running with some masters students from Latinoamerica in a few weeks time, I think is an amazing example for them to learn about Bayesian modelling. For this I'm going to translate the notebook to Spanish and will add it to the repo.

edaub commented 3 years ago

Yes, feel free to use this in future educational opportunities.

I will run the notebook again then and commit the final executed version. I won't commit a new HTML version and will delete it from the final commit as it sounds like that isn't needed. I'll let you know with a comment here that it is ready to merge.

samvanstroud commented 3 years ago

Hi @edaub, really sorry, I have jumped the gun a bit (not staying on top of this thread) and ran the notebook on my end and have pushed it here. If there are any problems let me know and I will roll back the changes (which also rename the notebook file as required by fastpages). If things look ok to you, then I think we can go ahead and merge this!

edaub commented 3 years ago

@samvanstroud No worries -- I have a couple of local changes that I haven't pushed but I will pull your changes and rebase on top of that. I presume I should make sure the final notebook gets moved to the path you specify for publication purposes?

samvanstroud commented 3 years ago

@edaub, sounds good. Yep., the timestamped path I introduce will help keep things organised on our end, thanks!

samvanstroud commented 3 years ago

@edaub, just a friendly nudge, it will be great to get this merged.

edaub commented 3 years ago

Thanks for the reminder -- the last week has been a bit crazy but I finally found an hour to read it over one last time. I just pushed the final version that I am happy with, so feel free to go ahead and merge/publish.

I'm happy to fix the conflicts, though I suspect you know better than I do which is the correct version so I will assume someone else is fixing this unless I hear otherwise.

Also, you can squash merge this if you don't want extra copies of the big notebook in the repo -- I don't need the previous versions hanging around for any reason, so up to you how you handle the merge.

samvanstroud commented 3 years ago

Ok, I resolved the conflicts and re-ran the notebook locally just for good measure (they appeared to be missing on my end). Merging now!