Skip to content

DOCS Updating the Rugby docs #2766

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

Merged
merged 1 commit into from
Jan 6, 2018
Merged

DOCS Updating the Rugby docs #2766

merged 1 commit into from
Jan 6, 2018

Conversation

springcoil
Copy link
Contributor

I've updated the Hierarchical Rugby model in the Documentation.

I've added - more data from more years, since before it only had 2014.

I've updated a few things in terms of the explanations, added a few more visualizations/ exploratory data analysis and added a few links to other resources.

I'd appreciate some proof reading, and for someone to verify if they run correctly.

@springcoil
Copy link
Contributor Author

Anyone know why this build is breaking? @twiecki @junpenglao ?

@ColCarroll
Copy link
Member

It looks like travis randomly killed everything... I restarted all builds

@ColCarroll
Copy link
Member

Oh, you might need to rebase off master: we pinned Theano to 0.9.0 for some reasons (#2728), but it looks like it is trying to install Theano 1.0.1 right now.

Notebook diffs are hard to see what changed! Generally there are a few edits I would make on the introduction. Feel free to ignore (especially ones that aren't associated with this change), and I can make a bunch of nits myself in a future PR.

  • First sentence should be something like

I came across the following blog post on Daniel Weitzenfeld's blog, based on the work of Baio and Blangiardo.

Note that I changed the link for the Baio and Blangiardo paper, since the old one is dead.

  • Remove the bullet points in the introduction, since most of them are not referred to later. The first (about which countries are in the Six Nations) is useful to know, but should be included in the previous sentence:

Since I am a rugby fan I decide to apply the results of the paper to the Six Nations Championship, which is a competition between Italy, Ireland, Scotland, England, France and Wales.

  • The last sentence of the introduction should be updated for the new data

  • The "Motivation" section might also be reworked. There is some capitalization which should not be there, and a stray close parens. I would drop the TODO as well.

  • You might reorder the data_csv_201X section to be chronological. You could also add a "year" column, and dump the csv in "pymc3/examples/data/rugby.csv", then use df_all = pd.read_csv(pm.get_data('rugby.csv'))

@springcoil springcoil force-pushed the DOCS-Rugby-Update branch 2 times, most recently from 4c7b074 to 4fba90c Compare December 29, 2017 15:51
@springcoil
Copy link
Contributor Author

I made those changes you suggested Colin.

@springcoil
Copy link
Contributor Author

If this builds correctly we can merge it I think. There's a few nitpicks etc, but I could fix them or @ColCarroll at some other point.

Making changes based on what Colin suggested

Forgot the rugby.csv file
@ColCarroll
Copy link
Member

This is really good - I like that we have a new data set! Feel free to merge whenever - you did not write any code that is run in the test suite, so travis is not going to say anything useful. (I am not merging right now because I am interested to see if it passes, or if travis is still unhappy for other reasons)

@springcoil
Copy link
Contributor Author

springcoil commented Dec 29, 2017 via email

@springcoil
Copy link
Contributor Author

springcoil commented Dec 29, 2017 via email

@AustinRochford
Copy link
Member

If you're using format instead of f-strings, you can take the lambda out of this expression

CONVERGENCE_TITLE = lambda: "BFMI = {}\nGelman-Rubin = {}".format(bfmi, max_gr)

@AustinRochford
Copy link
Member

Looks good overall! My only comment is that you may be able to avoid duplicating some of the model math in simulate_season with sample_ppc.

@springcoil
Copy link
Contributor Author

springcoil commented Jan 1, 2018 via email

@junpenglao junpenglao merged commit 586024d into master Jan 6, 2018
@junpenglao
Copy link
Member

Thanks @springcoil!

@junpenglao junpenglao deleted the DOCS-Rugby-Update branch January 6, 2018 15:26
@springcoil
Copy link
Contributor Author

Anyone know why the build is still failing?

@junpenglao
Copy link
Member

Travis CI is not building correctly in py2.7 on master for a while now. @twiecki will contact the Travis support.

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.

4 participants