idealo / imagededup

😎 Finding duplicate images made easy!
https://idealo.github.io/imagededup/
Apache License 2.0
5.18k stars 459 forks source link

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

Closed prateekralhan closed 1 year ago

prateekralhan commented 2 years ago

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

clennan commented 2 years ago

Thanks @prateekralhan, this looks awesome!

datitran commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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?

clennan commented 2 years ago

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!