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

Formatted and Updated README.md - Added Streamlit based WebApp 👨‍💻✅ #184

Closed
wants to merge 7 commits into from
Closed

Conversation

prateekralhan
Copy link

Hello @tanujjain / @clennan / @datitran, and https://github.com/idealo ,

Kudos to you for bringing up imagededup. I worked on developing a simple streamlit based webapp on the same and I think it will be fruitful to have it as a part of README here as the motivation behind developing this came from your work 😄! The entire webapp codebase has also been added as a separate folder while the main README.md of the project has been updated with the same.

You can find the entire webapp source-code in the stream_app directory of the repo.

Happy opensourcing!

Cheers,
Prateek

Created a simple streamlit based webapplication which can be deployed for finding duplicate images using imagededup in the backend
@clennan
Copy link
Collaborator

clennan commented Nov 23, 2022

Thanks @prateekralhan, this looks awesome!

@datitran
Copy link
Collaborator

hey @prateekralhan pretty cool. I did some formating already. Can you also please add a README.md, in particular on how you would run the streamlit app e.g. locally and also via Dockerfile. I also ran the Dockerfile. It's quite heavy with all the requirements.txt as well. Once this is done, we can test it and then merge it.

@tanujjain
Copy link
Collaborator

Hi @prateekralhan , thanks for you effort.

I'm not sure what the intent of the PR is. This looks like a streamlit app you built for your purpose using imagededup. Not sure how it adds to the functionality the package offers.

Could you clarify a bit more?

@prateekralhan
Copy link
Author

Thank you for your valuable comments and feedback gentlemen!

@datitran , Thank for making the formatting changes. I have added README.md in the streamlit_webapp folder as per your suggestion. Please have a look.

@tanujjain , You are completely correct. There is no as such added benefit or functionality added by me to the imagededup package. As mentioned before, I just created this webapp as a mere implementation of your work so that normal users can have some basic UI to access an app and use it for finding the duplicates since customer need is the end goal. I am ready to work more on this should we expect more updates with the package 😯🎉 !
Since the motivation for this came from your major chunk of work, I thought it would be fruitful to have it here. Hope this clarifies my thought process and mindset behind this. 😄

@tanujjain
Copy link
Collaborator

tanujjain commented Nov 23, 2022

Thanks for your explanation @prateekralhan .

Unfortunately, I can not agree with the logic of merging app-specific code into framework code. As a package, imagededup is expected to be a part of various codebases that use its functionality in whichever way the app developer sees fit (eg: like you creating a UI for your users, or someone writing a script to deduplicate a dataset for training an image recognition system, etc.). Merging such app-code into the base package doesn't serve much purpose. (Imagine the state of the package if all app developers push their application scripts into the base package.)

@datitran @clennan Please let me know if I'm missing something here.

@datitran
Copy link
Collaborator

datitran commented Nov 24, 2022

On a second thought, I agree with @tanujjain. Our library should only do one thing which is to provide other developers with a tool to find duplicated images. Applications should not be part of the repo. I would therefore suggest that @prateekralhan you should create a new repo in your user account e.g. imagededup-streamlit and host it there. Then we can just reference from the readme to this project which I still find pretty cool. And we would also promote this then. What do you think?

@tanujjain tanujjain changed the base branch from master to dev November 25, 2022 09:47
A simple streamlit based web application in order to find duplicates in a corpus of images using perceptual hashing, uploaded by the user. You can find the entire app's codebase in the ```streamlit_webapp``` folder :
A simple streamlit based web application in order to find duplicates in a corpus of images using perceptual hashing, uploaded by the user.

GitHub Repo : https://github.com/prateekralhan/Imagededup---Streamlit
Copy link
Collaborator

Choose a reason for hiding this comment

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

I just checked the link. Will you add it there @prateekralhan 🙈

Copy link
Author

Choose a reason for hiding this comment

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

@datitran , yes. I am yet to update my github repo with the necessary directories and files for the entire app's codebase. Just give me sometime please, I will update it and let you folks know so that the PR is good enough to be merged.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@datitran I would like to review the new repo before we add the link in our Readme.
@prateekralhan Could you please add the changes in your repo as a PR that I can review? (instead of adding all the remaining changes directly to the main branch. I see that right now you have only readme and license in there. So, maybe add me as a collaborator and allow me to review the changes you add in a new branch).

Copy link
Author

Choose a reason for hiding this comment

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

@datitran and @tanujjain I have updated the github repo's README. You can check it here.

@tanujjain , Apologies I just now checked the PR conversation here and I have already made the repo README updates in main branch itself directly. Please have a look and let me know should any changes be needed.

I have added all 3 of you as collaborators. You should have received the invites.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@prateekralhan I have submitted an issue with some suggestions. Please have a look and give us a note when you're ready.

Copy link
Author

Choose a reason for hiding this comment

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

Any updates? @tanujjain / @datitran / @clennan

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hi, I need to find some time for this. Will let you know soon.

Copy link
Collaborator

@tanujjain tanujjain Dec 20, 2022

Choose a reason for hiding this comment

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

@prateekralhan So, I went through the codebase again. Unfortunately, the code quality requires a lot of work to meet our standards, so we will not be able to include the link to the repo in our readme. We appreciate the work you have put in and the output still remains impressive and useful, so we will be happy to share your work on our social media handles if you fix one issue - You're using find_duplicates method, but you should be using find_duplicates_to_remove method to get a unique list of duplicate files, otherwise, you will have repetition in the duplicate files list. (Read about the methods here).

Please let us know if you're ok with this. If yes, please let us know when you have made the change and we will share the repo on our social media handles. Thanks for your understanding.

Copy link
Author

Choose a reason for hiding this comment

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

@tanujjain , thank you for the feedback. I have made the changes and updated the repo (https://github.com/prateekralhan/Streamlit-Imagededup).
I am fine with the social media sharing thing. 😊

Copy link
Collaborator

Choose a reason for hiding this comment

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

@prateekralhan Thanks for the update.
@datitran This looks good to me. We can share the link on our socials.

@clennan
Copy link
Collaborator

clennan commented Nov 25, 2022

Thanks for your explanation @prateekralhan .

Unfortunately, I can not agree with the logic of merging app-specific code into framework code. As a package, imagededup is expected to be a part of various codebases that use its functionality in whichever way the app developer sees fit (eg: like you creating a UI for your users, or someone writing a script to deduplicate a dataset for training an image recognition system, etc.). Merging such app-code into the base package doesn't serve much purpose. (Imagine the state of the package if all app developers push their application scripts into the base package.)

@datitran @clennan Please let me know if I'm missing something here.

Agreed @tanujjain - referencing the streamlit app in imagededup's Readme sounds like a great compromise!

@tanujjain tanujjain added this to To Do in Image Dedup Dec 27, 2022
@tanujjain tanujjain moved this from To Do to In Progress in Image Dedup Dec 27, 2022
@prateekralhan prateekralhan closed this by deleting the head repository Mar 18, 2023
@tanujjain tanujjain moved this from In Progress to Done in Image Dedup Apr 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Image Dedup
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

4 participants