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

Designed the landing page #68

Merged
merged 2 commits into from
Oct 17, 2020

Conversation

SoniGarima
Copy link

@SoniGarima SoniGarima commented Oct 14, 2020

The PR is for issue #57
I have added the following:

  1. Logo
  2. Slogan
  3. Names of all the classifiers included in this project ( Can add more later)
  4. Video illustration on how to use this web application

app.py Outdated
@@ -46,56 +45,71 @@ def main():

if choice == "Home":
Copy link
Owner

Choose a reason for hiding this comment

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

By default Home should be the first thing we visit when the app loads as well as in the Menu Bar on the left

Copy link
Author

Choose a reason for hiding this comment

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

Then what change should be done?

Copy link
Owner

Choose a reason for hiding this comment

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

The first if statement should be for Home Page

Copy link
Author

Choose a reason for hiding this comment

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

okay.

app.py Outdated
st.image(img)
st.image([covid_img,diabetic_img,heart_img,pneu_img,skin_img],caption=["COVID-19 Detection","Diabetic Retinopathy","Heart Disease Prediction","Pneumonia Detection","Skin Cancer Detection"])
st.write("Illustration of the Web Application:")
st.video('assets/demo.mp4')
Copy link
Owner

Choose a reason for hiding this comment

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

Can you please put a small recording of the app with this change in the description of this Pull Request

Copy link
Author

Choose a reason for hiding this comment

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

Yes, Here is the link.

Copy link
Owner

Choose a reason for hiding this comment

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

Thanks

app.py Outdated
st.video('assets/demo.mp4')

# below is the sample function you need to implement in separate python file

Copy link
Owner

Choose a reason for hiding this comment

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

Simply remove the code. If required we can define it in another script and import it

Copy link
Author

Choose a reason for hiding this comment

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

Okay.

@SoniGarima
Copy link
Author

I have done the required changes, could you please review?

@smaranjitghose smaranjitghose merged commit fce4e26 into smaranjitghose:test Oct 17, 2020
@pr-triage pr-triage bot added the PR: merged label Oct 17, 2020
@smaranjitghose
Copy link
Owner

give @SoniGarima 150 points

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

Successfully merging this pull request may close these issues.

2 participants