Skip to content

Customer churn - #3

Open
shreyasd2301 wants to merge 6 commits into
mainfrom
customer-churn
Open

Customer churn#3
shreyasd2301 wants to merge 6 commits into
mainfrom
customer-churn

Conversation

@shreyasd2301

@shreyasd2301 shreyasd2301 commented Apr 10, 2022

Copy link
Copy Markdown

created raw/raw_data_ingestion.py and and interim/interm_data_process.py
Added code in existing file of src/data/make_dataset.py

@ayush714 ayush714 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good work, I listed out some comments below and now one of the major comment which I want to list is formatting and styling, we recommend you to use black and isort in vs code to format your code to be consistent. You can check this blog for reference.

@@ -0,0 +1,42 @@
import pandas as pd

class DataPreprocessing:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please give a good doc string for this, It should be informative and professional.


class DataPreprocessing:

def __init__(self, train_data, transaction_data, user_logs_data, members_data) -> None:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add docstring with parameter information, like (below is just an example) :-

def train_model(
    x_train: pd.DataFrame,
    x_test: pd.DataFrame,
    y_train: pd.Series,
    y_test: pd.Series,
    config: ModelNameConfig,
) -> RegressorMixin:
    """It trains all the regression models and outputs the best model. 

    Args:
        x_train: pd.DataFrame
        x_test: pd.DataFrame
        y_train: pd.Series
        y_test: pd.Series
    Returns:
        model: RegressorMixin
    """

self.user_logs_data = user_logs_data
self.members_data = members_data

def drop_duplicates(self):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Add the same param information and doc string to every method and add error handling blocks as well and use rich print for that. https://rich.readthedocs.io/en/stable/introduction.html

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

(rich print)so basically you want to print params of every class or method in CLI

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes.

@@ -0,0 +1,15 @@
# https://www.technologiesinindustry4.com/2021/07/how-to-use-kaggle-api-in-python.html

import py7zr

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

same like above comments, wrap this in function or class.

raise e


if __name__ == "__main__":

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove this stuff.

@shreyasd2301 shreyasd2301 Apr 11, 2022

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

should i remove the 20th line.

or everything below it? what exactly?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

remove the if name == "main" and all things after it.

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