Fixes Docset for Discretize PR#742 #820
Conversation
|
@kris-singh Just fixing doc-tests will not suffice. |
|
@kris-singh You need to add tests for the changes you have made. |
|
@yashu-seth I was looking for the tests of Discretize methods i couldn't find them. Also, do you have any references for discretizing of multivariable Continuos Factors |
|
@kris-singh You have added the functionality to discretize univariate |
|
@yashu-seth Yes now discretization works for continuous factor. |
|
@kris-singh But only for univariate cases. So you will have to add tests for discretize method taking univariate Continuous Factors. |
|
@yashu-seth can you suggest some test. i was thinking about it. How can we check are we discretizing correctly or not. |
|
@kris-singh You can compute the values of probability at specific points using the pdf and check if you are getting the same values. |
|
@ankurankan @yashu-seth I have added the test.Are they enough? I am not able to understand travis-ci error how can values change when using python 3.4,.5 and python 2.7. |
|
@kris-singh Because there are a lot of changes in the behavior of python 2 and 3. Just look through your changes and see if you can see something that might be behaving differently in python 2 and 3. If are you unable to find something try running a debugger on both 2 and 3 and see where the values start differing. That will help you in locating the bug. |
1 similar comment
|
@ankurankan this is ready for review |
| np.testing.assert_almost_equal(results2, [-2.120911, -0.115055, 0.198042, 0.033288, 0.004727], decimal=4) | ||
| np.testing.assert_almost_equal(results3, [0.038335, 0.059015, 0.039992], decimal=4) | ||
| np.testing.assert_almost_equal(results3, [0.008325, 0.028291, 0.030074], decimal=4) |
There was a problem hiding this comment.
@kris-singh Why did these values change ?
There was a problem hiding this comment.
changes made to division made in python3.
for python2.7 i used from future import division
There was a problem hiding this comment.
@kris-singh But that does mean that the tests were either wrong earlier or are now
There was a problem hiding this comment.
i think i have to figure this one out. I will check my calculations again.
There was a problem hiding this comment.
@ankurankan Yes i checked the calculations. Both python 2.7 and 3.5 give the ans. The only trouble was i wrote 5/2 in python 2.7 gamma(5/2) = 1 if gamma(5.0/2.0) = 1.3293. division in python3.5 are by default are float.
|
@ankurankan ready for review |
There was a problem hiding this comment.
@kris-singh There is something wrong with the implementation. The discretized values shouldn't be negative as it's a probability distribution.
|
@ankurankan i will check the problem out. |
No description provided.