berkeley-stat222 / mousestyles

2016 final project
http://berkeley-stat222.github.io/mousestyles/
BSD 2-Clause "Simplified" License
2 stars 33 forks source link

WIP: fix typo #178

Closed jarrodmillman closed 8 years ago

jarrodmillman commented 8 years ago

@lunadadada I fixed a few errors from your pull request. Please check that I haven't messed anything up and let me know if I missed any other errors in your script.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.04%) to 60.924% when pulling edf1e74a28dbcc8198ddd3183f4edbfe06e35ae2 on jarrodmillman:fix_error into fe934f35e67546a66e3cc71c8f58aec6ea14211d on berkeley-stat222:master.

lunadadada commented 8 years ago

@jarrodmillman Sorry about all my mistakes. Thank you for all correction for me and I have checked all codes. There are two things I found maybe problems:

  1. Since the function name plot_expoential() is changed to plot_exponential(), when I call the function plot_expoential(Estimation) in the sample script plot_distribution.py, it should also be changed to plot_exponential(Estimation).
  2. Also in the plot_distribution.py, there is a typo from mousestypes.est_power_param import fit should be changed to from mousestyles.est_power_param import fit. Do I have the access to fix these two problems and push to your branch jarrodmillman:fix_error?
jarrodmillman commented 8 years ago

@lunadadada , thanks for finding the extra issues. I also found several other issues with your script.

You never use these imports:

import matplotlib.pyplot as plt
from mousestyles import data

You also never import, but you do use:

plot_powerlaw
plot_exponential
plot_fitted

You also capitalized Estimation when it should be lowercase. And you didn't have spaces around the assignment.

Finally, you have the following this line gives an ImportError

from mousestyles.est_power_param import fit

since there is no function named fit in mousestyles.est_power_param. How should I fix this for you?

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.04%) to 60.924% when pulling 6089a39220b679d1cb16d3d285dfd472e2329bc5 on jarrodmillman:fix_error into fe934f35e67546a66e3cc71c8f58aec6ea14211d on berkeley-stat222:master.

jarrodmillman commented 8 years ago

The idea of the plot directive is that is allows you to include a plot for the report using the actual code that generates the plot. Here is an example: http://berkeley-stat222.github.io/mousestyles/report/path.html

Is your script intended to generate one plot or several? If it generates several, does each plot function you call generate one plot? If so, I will create 3 separate scripts: one for each plot that you want to include. If you have suggestions for names for the scripts, please let me know.

coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.04%) to 60.924% when pulling 852ca6321b8c21d032cbeb9924c2aaa23016b881 on jarrodmillman:fix_error into fe934f35e67546a66e3cc71c8f58aec6ea14211d on berkeley-stat222:master.

lunadadada commented 8 years ago

@jarrodmillman We are supposed to insert several plots in our report so I have divided the script into three and fixed the import error. Please review and give me your suggestion. If it is okay, you can delete the old one and add these three on the plots folder.

plot_powerlaw.py

from mousestyles.est_power_param import fit_dist_all
from mousestyles.distribution_plot import plot_powerlaw

estimation=fit_dist_all()

# Plot the histogram of the parameters of powerlaw 
plot_powerlaw(estimation)

plot_exponential.py

from mousestypes.est_power_param import fit_dist_all
from mousestyles.distribution_plot import plot_exponential

estimation=fit_dist_all()

# Plot the histogram of the parameters of exponential
plot_exponential(estimation)

plot_fitted.py

from mousestyles.distribution_plot import plot_fitted

# Plot the histogram and fitted curve for strain 0, mouse 2, day 5
plot_fitted(0, 2, 5)
coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.04%) to 63.107% when pulling ea197e6bc072d22757bc8aa2ce796345e22ed79e on jarrodmillman:fix_error into 9ae3ba67bcbf53289ee77534326f984a07c6537c on berkeley-stat222:master.

jarrodmillman commented 8 years ago

@lunadadada I've made the changes and after the test pass I will merge this.

I was hoping that I would be able to copy-and-paste your new code, but it also had errors in it that I had to fix before the code would run. Next time, please take care to provide code that you have tested to ensure that it is correct. Here are a couple issues with the last code:

from mousestyles.distribution_plot

should be

from mousestyles.visualization.distribution_plot

And

from mousestypes.est_power_param import fit_dist_all

should be

from mousestyles.est_power_param import fit_dist_all

Finally, this a style issue (but one I corrected you on in this pull request already), but you are supposed to put spaces around the assignment operator like this:

estimation = fit_dist_all()
coveralls commented 8 years ago

Coverage Status

Coverage decreased (-0.04%) to 63.107% when pulling afcde95f731f2842d7fc4ef5f7d647ee8dc582a5 on jarrodmillman:fix_error into 9ae3ba67bcbf53289ee77534326f984a07c6537c on berkeley-stat222:master.