Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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 #75

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

rishabh-jain14
Copy link

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?

@rishabh-jain14 rishabh-jain14 mentioned this pull request Aug 31, 2021
@rishabh-jain14 rishabh-jain14 changed the title 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 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 Aug 31, 2021
@ddbourgin
Copy link
Owner

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
Copy link
Author

rishabh-jain14 commented Sep 23, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants