Closed gyutaepark closed 6 years ago
Hello @gyutaepark! Thanks for updating the PR.
analysis/analysis.py
, following are the PEP8 issues :Line 519:9: E722 do not use bare except'
I'll be adding another commit that I've forgotten about but is relevant to this PR.
Addresses #25
Readme.ipynb is just markdown, without code or results. If it's a notebook, then it should be runnable. When I suggested this, I hoped for something somewhat like a combination of analysis.ipynb and readme.ipynb. Ideally, a single notebook should act like a runnable report. It should have a title, introduction, assumptions, then cells for the code where you input those assumptions, a description of the scenario (and then again cells for function calls where you generate the input file), and then it should display the results and discuss them.
Here's an example of a runnable paper that was set up in this way : http://nbviewer.jupyter.org/github/cossatot/lanf_earthquake_likelihood/blob/master/notebooks/lanf_manuscript_notebook.ipynb .
Of course, yours doesn't have to be so involved or have so much text, but the above is what I'm generally envisioning.
Thanks for the review! I'll update the report to be a little more interactive.
Sorry for the delay but I have finally finished reviewing the README.ipynb to be more interactive and this should be ready for a review.
The report is now ready for a review. Thank you
Good job @gyutaepark!
print
in the notebook make it hard to read ( namely the composition and reactor data, maybe format them in a way that it is presentable (reduce sigfigs, or make a plot) or get rid of them entirely? )Thanks @jbae11 ! I'm currently improving the print statements in most items so that only parts of the variables are printed in a much more readable manner, and also removed a couple of print statements that do not have much meaning and are displayed as plots later.
I am requesting a new review to see if the print results are acceptable @jbae11 Thank you!
Just talked with Teddy for a new review. He finds it ready for merging. Now requesting a review from @katyhuff . Thank you!
I have merged this PR since the report is completed and seems to have no errors.
The analysis and the report is now completed for the "US" region and requires a review. The report is available in
README.ipynb
.