-
-
Notifications
You must be signed in to change notification settings - Fork 56
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
Create GUI #70
Create GUI #70
Conversation
Prakhar314
commented
Oct 14, 2020
•
edited
edited
- Frame the website using HTML and Flask
- Connect python modules. Finish basic functionality.
- Work on styling.
@Prakhar314 Looks good, Please also add how to use the Web app. Better add a section in Readme about telling the working of this web app so others can contribute to this web app also. Feel free to add credits for the web app in the readme to yourself. |
@Prakhar314 Being Dynamic webpage, will it be possible to host it somewhere? I think gitpages only supports static pages for now. |
@Prakhar314 Added WIP in title for now, please remove that once you are done and let me know so that I can review and merge this. |
62d224a
to
e197e2e
Compare
Heroku could be used I think. |
I think I'm done, please review. |
@Prakhar314 Thanks, let me Review. |
// //get contributors | ||
// request = new XMLHttpRequest(); | ||
// request.open('GET', 'https://api.github.com/repos/ritwik12/Celestial-bodies-detection/contributors'); | ||
// request.send(); | ||
// request.onload = async function () { | ||
// var data = JSON.parse(this.response); | ||
// console.log(data); | ||
// var ih = ""; | ||
// // var num=0; | ||
// data.forEach(element => { | ||
// if (!element.login.includes("bot")) { | ||
// // num++; | ||
// ih = ih.concat("<a href=\"", element.html_url, "\" target=\"_blank\">", element.login, "</a>"); | ||
// } | ||
// }); | ||
// document.getElementById('num_contributors').innerHTML = "Contributors"; | ||
// document.getElementById('contributors').innerHTML = ih; | ||
// } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Prakhar314 I think we can remove this.
<script> | ||
// send request for project description | ||
console.log("yp"); | ||
var request = new XMLHttpRequest(); | ||
request.open('GET', 'https://api.github.com/repos/ritwik12/Celestial-bodies-detection'); | ||
request.send(); | ||
// set description | ||
request.onload = async function () { | ||
var data = JSON.parse(this.response); | ||
console.log(data.description); | ||
document.getElementById('aboutProject').innerHTML = data.description; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Prakhar314 I see you are currently fetching the Description of repo. This is good but very limited. We can add more details about the project and some cool images to give more info and look better. What do you say?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can. What do you want to be shown?
I was showing the list of contributors, but commented it out for now (the above review).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Prakhar314 We can show contributors as well, but we should show something about the project in detail. You can use anything from Readme.md or summarise based on the understanding. Basically it should give an idea to the people coming to the website about the Project and what it does.
app/templates/index.html
Outdated
<div class="my-image-form"> | ||
{{form.image_file}} | ||
<div class="custom-file" id="custom-file-upload"> | ||
<div id="file-label" for="customFile" onclick="open_browser();" class="custom-button text-center">Choose |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Prakhar314 I think instead of Choose file
we can change this to Choose Celestial Image
to be more cool.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any other suggestions?
app/templates/index.html
Outdated
</div> | ||
<div class="my-image-form col-12 d-flex justify-content-center"> | ||
<div class="my-image-form"> | ||
<button type="submit" onclick="upload_image();" class="custom-button">Upload</button> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Prakhar314 So, this upload button is not actually uploading the image but actually works as a SUBMIT or ENTER button for an uploaded image or entered URL. How about changing it to something like Classify
or any other suggestions for the word?
app/views.py
Outdated
from app import app | ||
from flask import render_template, request, redirect, jsonify, make_response, url_for | ||
from werkzeug.utils import secure_filename | ||
from base64 import b64encode | ||
import os | ||
import tensorflow as tf | ||
from hub.examples.image_retraining.label_image import get_labels, wiki | ||
from hub.examples.image_retraining.reverse_image_search import reverseImageSearch | ||
from flask_wtf import FlaskForm | ||
from flask_wtf.file import FileField, FileAllowed, FileStorage | ||
from wtforms import StringField, validators | ||
|
||
from PIL import Image | ||
import requests | ||
from io import BytesIO |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Prakhar314 For a better habit always try to follow the PEP8 standards. Most of the projects are strictly restricted to PEP8. For more info: https://www.python.org/dev/peps/pep-0008/#imports
app/views.py
Outdated
image_url = StringField( | ||
"image_url", | ||
validators=[validators.Optional(), validators.URL()], | ||
render_kw={"placeholder": "Enter a URL"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Prakhar314 Fine with Enter a URL
too, but any other suggestions to make it more cool?
@Prakhar314 Appreciate your hard work here, Amazing contribution. |
@Prakhar314 I can see that you have not made any changes to a lot of files but still they are showing in the Diff for your PR. Can you please fix that? |
@Prakhar314 You have done an amazing job here putting up the GUI and I think with the GUI a more people will be able to use and contribute to the project. I am really interested in hosting the Web App too. Will you be able to work on that too? |
I can't figure out how to do that. Do you know how to? |
@Prakhar314 so when you are doing Whenever you are working on something which is not a small contribution then always create a new branch and push changes to that only. This helps in cases like these to revert things and fix them. For now if you want, you will have to store your changes to some other places, then hard reset your master branch which will make it back to head of the upstream. Then make the changes and pushing them. Be aware that when you reset the master it will not have any diff which eventually will close this PR 😅. so for now I think you can go like this only. There are no issues. |
I think https://cloudfour.com/thinks/quick-tip-how-to-hide-whitespace-changes-in-git-diffs/ is for the reviewer only while reviewing to hide those. You can also hide it but for your VIEW only not for others. And Hiding it does not make it get away from there. They are there. |
@Prakhar314 Please check the reviews and let me know if you have any doubt. Hope you are not in a hurry, if you have worked hard I want it to be perfect at last. |
@Prakhar314 Any updates here? |
I rebased and force pushed to remove whitespace changes. I also changed some of the button names as asked. |
@Prakhar314 Ohk sure, Thanks :) |
@Prakhar314 Did you got time to update? Let's just add screenshots to Readme and Merge this one. |
@Prakhar314 Reminder!! |
@Prakhar314 Final nudge, Lets just resolve conflicts to this PR and merge it. we can work on improvements later. |
@Prakhar314 Merged this, thanks!! |