Skip to content

Conversation

sudipg4112001
Copy link
Contributor

Related Issue

Fixes #510

Copy link
Contributor

@kaustubhgupta kaustubhgupta left a comment

Choose a reason for hiding this comment

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

Just a few modifications before merging:

  • Add a section in readme named "Setup Instructions" where describe that extract the dataset.rar and how to run the script.
  • Update all the paths in script as shown below in example:
testing_target=data_set('./Face-Mask-Detection/kaggle/input/face-mask-detection/test/')
  • Shift all the relevant import statements on top

@kaustubhgupta kaustubhgupta added the minor-change-not-bug Suggested a minor change in code, not a bug label Mar 17, 2021
@sudipg4112001
Copy link
Contributor Author

I have made the necessary changes. Please review it.

Copy link
Contributor

@kaustubhgupta kaustubhgupta left a comment

Choose a reason for hiding this comment

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

Run the whole script on your system and debug the errors. Push your changes once you are confirmed that the script runs and there is no error from your side. Found some while reviewing only:

from sklearn.metrics import classification_report
import sklearn.metrics as metrics
import itertools
for dirname, _, filenames in os.walk('/kaggle/input'):
Copy link
Contributor

Choose a reason for hiding this comment

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

This path also needs modification

data = np.array(data, dtype="float") / 255.0
target = tf.keras.utils.to_categorical(np.array(target), num_classes=2)
return data, target
training_target=data_set('./Face-Mask-Detection/kaggle/input/face-mask-detection/train/')
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you want to load here? There is no extension to files. Same for next testing_target and valid_target. Also, where are the testing_data, training_data, and valid_data variables? They are not initialized anywhere but passed into functions.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kaustubhgupta I think he is specifying the file here , I could be wrong as I haven't run this code yet

Copy link
Contributor

Choose a reason for hiding this comment

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

Actually the pylint test gave these errors and when I saw the script, I found these variables are missing but passed in functions

Copy link
Contributor

Choose a reason for hiding this comment

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

@kaustubhgupta Yup , the linter is giving error because of undefined variables. I am referring to the training_target variable , he is taking directory as argument to data_set function and then appending file name here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you want to load here? There is no extension to files. Same for next testing_target and valid_target. Also, where are the testing_data, training_data, and valid_data variables? They are not initialized anywhere but passed into functions.

extension is not required, because they are folders and they will directly have access to the files within them.

@kaustubhgupta kaustubhgupta added bug Something isn't working and removed minor-change-not-bug Suggested a minor change in code, not a bug labels Mar 18, 2021
@sudipg4112001
Copy link
Contributor Author

What should I do ?

@kaustubhgupta
Copy link
Contributor

What should I do ?

all the suggestions are mentioned above

@sudipg4112001
Copy link
Contributor Author

I have done the changes, please do review

@kaustubhgupta
Copy link
Contributor

@antrikshmisri I ran the script but it didn't work. What should we do?

@antrikshmisri
Copy link
Contributor

antrikshmisri commented Mar 19, 2021

@antrikshmisri I ran the script but it didn't work. What should we do?

@kaustubhgupta I am unable to get some dependencies so , couldn't test it. @santushtisharma10 Can you confirm the same

@sudipg4112001
Copy link
Contributor Author

I need to do something ?

@santushtisharma10
Copy link
Contributor

@antrikshmisri @kaustubhgupta facing following error while running the script
image

@kaustubhgupta
Copy link
Contributor

@antrikshmisri @kaustubhgupta facing following error while running the script
image

@sudipg4112001 Can you guide how to fix this? Is the script running on your machine? Can you share any GIF, video as Proof?

@kaustubhgupta kaustubhgupta added hold Needs a second thought and removed bug Something isn't working labels Mar 20, 2021
@sudipg4112001
Copy link
Contributor Author

sudipg4112001 commented Mar 20, 2021

Yes, this same code is running smoothly in my device. I am confused what's the problem with yours. I am attaching a screenshot below.

Face-Mask-Detection-ss

After making the changes you suggested it's now running smoothly.

@kaustubhgupta
Copy link
Contributor

Yes, this same code is running smoothly in my device. I am confused what's the problem with yours. I am attaching a screenshot below.

Face-Mask-Detection-ss

After making the changes you suggested it's now running smoothly.

As he has shown this screenshot, should we approve this? @antrikshmisri @santushtisharma10

@antrikshmisri
Copy link
Contributor

Yes, this same code is running smoothly in my device. I am confused what's the problem with yours. I am attaching a screenshot below.
Face-Mask-Detection-ss
After making the changes you suggested it's now running smoothly.

As he has shown this screenshot, should we approve this? @antrikshmisri @santushtisharma10

@kaustubhgupta @santushtisharma10 Okay from my side but I think we should refer @avinashkranjan first.

@kaustubhgupta
Copy link
Contributor

Yes, this same code is running smoothly in my device. I am confused what's the problem with yours. I am attaching a screenshot below.
Face-Mask-Detection-ss
After making the changes you suggested it's now running smoothly.

As he has shown this screenshot, should we approve this? @antrikshmisri @santushtisharma10

@kaustubhgupta @santushtisharma10 Okay from my side but I think we should refer @avinashkranjan first.

Yes, that makes sense

@kaustubhgupta
Copy link
Contributor

Updates: Conveyed the message to PA, he will respond soon

@avinashkranjan
Copy link
Owner

The Scripts Seems to working for me..
Just Correct This Error
Screenshot 2021-03-22 000445

I Just Commented out line 39..And the Script works fine for me
Screenshot 2021-03-22 000750

So, I'm approving the Script..Make the required changes in line 39, Else Remove it..Will Merge it then

@sudipg4112001
Copy link
Contributor Author

I have made the changes..

@sudipg4112001
Copy link
Contributor Author

Do review

@avinashkranjan
Copy link
Owner

@sudipg4112001 Don't Spam the Comments..Will Review it in Sometime..

Copy link
Owner

@avinashkranjan avinashkranjan left a comment

Choose a reason for hiding this comment

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

LGTM..💯🚩

@avinashkranjan avinashkranjan added Approved PR Approved and Ready to Merge gssoc23 Issues created for/by the GirlScript Summer of Code'23 Participants level3 New features, Major bug fixing and removed hold Needs a second thought labels Mar 21, 2021
@avinashkranjan avinashkranjan merged commit cae89d8 into avinashkranjan:master Mar 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Approved PR Approved and Ready to Merge gssoc23 Issues created for/by the GirlScript Summer of Code'23 Participants level3 New features, Major bug fixing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Mask or not

5 participants