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

Module Development, changed doc string as required, two new Jupyter notebooks #5

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

MuhangTian
Copy link

Finished T-test and relevant tutorial for T-test.py

KMeans.py Outdated
Function that helps to determine how many clusters to use by using trials of K clusters
The idea is to find the cluster number that gives the maximum reduction in inertia
'''
def elbow_method(data, num_k, n_init=10, max_iter=300):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

considering adding "saving the generated elbow plot" option ; (i.e. pass another argument "save" with default save = False )

KMeans.py Outdated
from sklearn.cluster import KMeans
from sklearn.datasets import make_blobs

'''

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please refer to the style guide for the conventions we are hoping to follow (https://numpydoc.readthedocs.io/en/latest/format.html#short-summary). block comments below def statement

import numpy as np
import matplotlib.pyplot as plt

'''

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refer to KMeans.py for annotations on code conventions we want to follow

Copy link

@hayoung-jeong hayoung-jeong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hayoung's comments for model development team

@MuhangTian MuhangTian changed the title Module Development, finished T-test Module Development, changed doc string as required, two new Jupyter notebooks Jul 21, 2022
@MuhangTian
Copy link
Author

Added two ML Jupyter notebooks, one with ML regression and the other with unsupervised clustering functions from scikit-learn

Copy link

@hayoung-jeong hayoung-jeong left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed and commented-- major changes: ipynb --> py files

README.md Outdated
# ML Models
* See jupyter notebooks, it contains everything the user need to know in the doc strings

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe a link to the jupyter notebook would be more intuitive for the users :)

"metadata": {},
"outputs": [],
"source": [
"class LinearRegression():\n",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If class is already defined as another py file, I would import and call instead of having another cell.

@@ -0,0 +1,347 @@
{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As these are more function definitions (or class as you are creating), I would have created a ml_regression.py file, and define them there. I think we envision Jupyter notebooks to be used as sample codes/visualization of how your functions work :)

README.md Outdated
# ML Models
* See jupyter notebooks, it contains everything the user need to know in the doc strings

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

consider putting a link for the users-- probably more intuitive 💡

@@ -0,0 +1,187 @@
{

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As commented on ml_regression.ipynb, could we have this as another py file.

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