In response to Issue #67: Naive Bayes Classifier added along with a unit testing file; also, included a test_use_cases.py file to compare the accuracy of naive bayes models using both numpy-ml and scikit-learn using dataset in the file wine.data

rishabh-jain14 commented 3 years ago

All Submissions

[Yes] Is the code you are submitting your own work?
[Yes] Have you followed the contributing guidelines?
[Yes] Have you checked to ensure there aren't other open Pull Requests for the same update/change?

New Model Submissions

[Yes] Is the code you are submitting your own work?
[Yes] Did you properly attribute the authors of any code you referenced?
[Yes] Did you write unit tests for your new model?
[Yes] Does your submission pass the unit tests?
[No] Did you write documentation for your new model?
[No] Have you formatted your code using the black deaults?

Changes to Existing Models

[Yes] Have you added an explanation of what your changes do and why you'd like us to include them?
[Yes] Have you written new tests for your changes, as applicable?
[Yes] Have you successfully ran tests with your changes locally?

ddbourgin commented 3 years ago

Hi Rishabh, thanks for the PR, and sorry for my delayed response. Also apologies for not closing issue #67 after @sfsf9797 's PR. I suspect this may have created some confusion :-/

Can you explain what your code adds over the existing implementation? I haven't gone through it in detail, but given that there already exists a validated Gaussian naive Bayes model in the repo, I'm inclined to leave things as they are unless there is significant added functionality.

rishabh-jain14 commented 3 years ago

Hey David, thanks for the response.

The PR focuses on the implementation and comparison of the Gaussian Naive Bayes Classifier using the NumPy-ml repo as well as the scikit-learn library.

I had realized that the repo already contains a code for the Gaussian naive Bayes model but as per the issue mentioned #67, it required a brief coded file that focused on the comparison of the performance of the model in comparison to the already specified scikit-learn model. So, we can say that the comparison part is the significant added functionality here.

Feel free to include the PR into the repo if you'd like. I enjoyed assessing the entire repository and look forward to contributing as much as I can. :-)

Rishabh Jain

On Thu, Sep 23, 2021 at 9:04 PM David Bourgin @.***> wrote:

Hi Rishabh, thanks for the PR, and sorry for my delayed response. Also apologies for not closing issue #67 https://github.com/ddbourgin/numpy-ml/issues/67 after @sfsf9797 https://github.com/sfsf9797 's PR. I suspect this may have created some confusion :-/

Can you explain what your code adds over the existing implementation? I haven't gone through it in detail, but given that there already exists a validated Gaussian naive Bayes model in the repo, I'm inclined to leave things as they are unless there is significant added functionality.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/ddbourgin/numpy-ml/pull/75#issuecomment-925926417, or unsubscribe https://github.com/notifications/unsubscribe-auth/ANHP7HB3YYKK6WHSMC76T6TUDNCJ3ANCNFSM5DD2B7IQ .

ddbourgin / numpy-ml