Skip to content

zahrasa: ready for review#42

Open
zahrasa wants to merge 2 commits intomainfrom
content_zahrasa
Open

zahrasa: ready for review#42
zahrasa wants to merge 2 commits intomainfrom
content_zahrasa

Conversation

@zahrasa
Copy link
Collaborator

@zahrasa zahrasa commented Apr 30, 2023

No description provided.

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@zahrasa zahrasa added the Science data science tasks label Apr 30, 2023
@@ -0,0 +1,2717 @@
{
Copy link
Collaborator

@soroushheidary99 soroushheidary99 May 1, 2023

Choose a reason for hiding this comment

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

Line #46.                                  content_list.append(str(content)) # example -> content: hello world!

each feature could have a separate function for extraction (for code readability and stuff :D)


Reply via ReviewNB

@@ -0,0 +1,2717 @@
{
Copy link
Collaborator

@soroushheidary99 soroushheidary99 May 1, 2023

Choose a reason for hiding this comment

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

maybe better to sample a limited number of rows of each news site instead of containing full agencies but full row sampling...


Reply via ReviewNB

@@ -0,0 +1,2717 @@
{
Copy link
Collaborator

@soroushheidary99 soroushheidary99 May 1, 2023

Choose a reason for hiding this comment

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

plain neural networks are extremely vulnerable to class domination, unless using balancing techniques or losses


Reply via ReviewNB

@@ -0,0 +1,2717 @@
{
Copy link
Collaborator

@soroushheidary99 soroushheidary99 May 1, 2023

Choose a reason for hiding this comment

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

using a context manager instead of closing manaully?


Reply via ReviewNB

@@ -0,0 +1,2717 @@
{
Copy link
Collaborator

@ghazaleh-mahmoodi ghazaleh-mahmoodi May 1, 2023

Choose a reason for hiding this comment

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

When we have an unbalanced data set, using the f1-score instead of accuracy can be a good choice for the model metric.


Reply via ReviewNB

@@ -0,0 +1,2717 @@
{
Copy link
Collaborator

@ghazaleh-mahmoodi ghazaleh-mahmoodi May 1, 2023

Choose a reason for hiding this comment

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

Line #11.        for i in tqdm(range(len(news_df['html']))):

When we use the pandas data frame, using .iterrows() is easier and readable than writing a for loop on range(len(df)).


Reply via ReviewNB

@@ -0,0 +1,2717 @@
{
Copy link
Collaborator

@ghazaleh-mahmoodi ghazaleh-mahmoodi May 1, 2023

Choose a reason for hiding this comment

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

Line #70.        if training_mode == False:

else: :)


Reply via ReviewNB

@@ -0,0 +1,2717 @@
{
Copy link
Collaborator

@ghazaleh-mahmoodi ghazaleh-mahmoodi May 1, 2023

Choose a reason for hiding this comment

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

Line #81.        if training_mode == True:

Two consecutive for loops can be written in the form of a for loop with two AND conditions


Reply via ReviewNB

@@ -0,0 +1,2717 @@
{
Copy link
Collaborator

@ghazaleh-mahmoodi ghazaleh-mahmoodi May 1, 2023

Choose a reason for hiding this comment

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

Great job, keep it up.


Reply via ReviewNB

Copy link
Collaborator

Choose a reason for hiding this comment

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

@zahrasa zahrasa changed the title zahrasa: ready to review zahrasa: ready for review May 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Science data science tasks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants