Closed FengshiNiu closed 8 years ago
@shamindras could you please do a quick code review?
@FengshiNiu - just did a code review. Make sure all Travis tests pass before addressing any suggested changes
@shamindras thanks for the comments. These are really good suggestions for coding style. I have just modified the code
@FengshiNiu When you are ready for me to merge this, rebase to upstream master and change the title to MRG from WIP.
Sure. I am still working on it and will change to MRG as soon as I finish it
On Fri, May 13, 2016 at 11:34 PM, Jarrod Millman notifications@github.com wrote:
@FengshiNiu https://github.com/FengshiNiu When you are ready for me to merge this, rebase to upstream master and change the title to MRG from WIP.
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/berkeley-stat222/mousestyles/pull/199#issuecomment-219204076
Fengshi Niu PhD student Department of Economics University of California, Berkeley fniu@berkeley.edu
@jarrodmillman ready to merge!
@FengshiNiu Please rebase on the upstream master and I will merge this. Let me know if you don't know how to rebase and I will do it for you.
Also did the your results (from this code) make into the report?
@jarrodmillman I am finishing adding results in the report now.
@jarrodmillman I wil add the modification of report in this pull request
@FengshiNiu OK
@shamindras Thank you very much for help
@jarrodmillman I have just added the result to the report. I tried hard but the 2 plots don't show up. Could you have a look at it? The plot script called are report/plots/plot_kernel_smoothing.py and report/plots/plot_relative_distribution.py
@FengshiNiu When I do the following
$ cd doc/source
$ make html
your plots are generated and included in the generated HTML. I suspect that you may have something misconfigured on your computer or that you are expecting GitHub to know how to generate figures using the code in our repository (GitHub doesn't render things based on the code in our project, but using their own code which may not have all the functionality our code has).
This looks OK to merge once the tests finish. If you want me to wait for you to do something else, please let me know. Otherwise, I will merge this once the tests complete (assuming they pass).
Thanks for the clarification. I've finished the work. It's ready to merge if it passes the tests.
On Saturday, May 14, 2016, Jarrod Millman notifications@github.com wrote:
@FengshiNiu https://github.com/FengshiNiu When I do the following
$ cd doc/source $ make html
your plots are generated and included in the generated HTML. I suspect that you may have something misconfigured on your computer or that you are expecting GitHub to know how to generate figures using the code in our repository (GitHub doesn't render things based on the code in our project, but using their own code which may not have all the functionality our code has).
This looks OK to merge once the tests finish. If you want me to wait for you to do something else, please let me know. Otherwise, I will merge this once the tests complete (assuming they pass).
— You are receiving this because you were mentioned. Reply to this email directly or view it on GitHub https://github.com/berkeley-stat222/mousestyles/pull/199#issuecomment-219237126
Fengshi Niu PhD student Department of Economics University of California, Berkeley fniu@berkeley.edu
Contributing issue #193. Add gaussian kernel estimation of the distance distribution, add the estimated distribution to the plots in the review. @doutib @hduongtrong @CenzhuoYao @z357412526 @lunadadada please review