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

Drop images #77

Open
ahmedbesbes opened this issue Dec 6, 2019 · 16 comments
Open

Drop images #77

ahmedbesbes opened this issue Dec 6, 2019 · 16 comments
Assignees
Labels
ms-beta Part of make-sense beta efforts new-feature New feature or request

Comments

@ahmedbesbes
Copy link

Would it be possible to drop some noisy images from the dataset?

@SkalskiP
Copy link
Owner

Hi, @ahmedbesbes! We don't have this functionality yet. But you can expect it to enter MakeSense in the next development round. At the moment I only have hint that if you don't make labels for a given photo then you won't be exporting any file that would concern it. This is not deletion.... I know. But at least you don't have garbage.

@SkalskiP SkalskiP self-assigned this Dec 10, 2019
@SkalskiP SkalskiP added new-feature New feature or request idea labels Dec 10, 2019
@TKassis
Copy link

TKassis commented Feb 16, 2020

I have a related suggestion. I'd like to be able to include a skipped image in the export. A few object detection models I'm using accept negative images (i.e. images with no objects of interest). Is this possible?

@SkalskiP
Copy link
Owner

@ahmedbesbes I'm going to implement that feature in the upcoming version of MakeSense.

@SkalskiP SkalskiP pinned this issue May 26, 2021
@SkalskiP SkalskiP added ms-beta Part of make-sense beta efforts and removed idea labels Aug 27, 2021
@SkalskiP SkalskiP added this to To do in make-sense beta via automation Aug 27, 2021
@rasyidf
Copy link
Contributor

rasyidf commented Jul 26, 2022

I'll try to take this.

@SkalskiP
Copy link
Owner

@ahmedbesbes let me know if you have any problems

@rasyidf
Copy link
Contributor

rasyidf commented Jul 27, 2022

I wanted to add this functionality, but needed a new settings parameter, wondering where exactly do you suggest adding this toggle?

@SkalskiP
Copy link
Owner

I'll try to prepare some task requirements today. ;)

@SkalskiP
Copy link
Owner

Hi, @rasyidf 👋! Below you will find guidelines for this task. I tried not only to answer your question but also to point out some problems that may arise.

  • Functionality should be to irreversibly remove the photo from the dataset. So not toggle (on / off) but delete button.

  • User should be able to trigger the deletion of the photo in two ways.

    • There should be a delete button at the top, that would enable to delete of the currently annotated photo. The button should use the trash icon. Please also add the separator between buttons on the left and the newly added delete button. The delete button should display a tooltip on the hover saying: "Delete current image".

    Screenshot 2022-07-27 at 12 50 39

    • There should be an additional delete button for every image in the left panel. This button should be only visible on hover. The button should be #d42245 with the white cross.

    Screenshot 2022-07-27 at 12 59 39

  • When a user tries to delete an image that contains annotation, we need to display a confirmation popup. With the question: "You are about to delete the image that contains {annotations_type} annotations. Would you like to continue?". We have a reusable component GenericYesNoPopup that you can leverage. A good example of using that popup is in ExitProjectPopup. annotations_type should match the type of annotations that would be removed - box, polygon, line, or point.

  • We should be able to figure out that the image contains labels based on logic similar to the ImagesList.isImageChecked method, but we need to check for all of them, not for activeLabelType.

  • When the user removes the current image, we should make sure that the next image from the list would be loaded. You can get current image index calling LabelsSelector.getActiveImageIndex() and you can load different image by calling updateActiveImageIndex.

@rasyidf
Copy link
Contributor

rasyidf commented Jul 27, 2022

noted, I'll work on it as soon as I'm done with my engagement party.

@SkalskiP
Copy link
Owner

🎉 congratulations!!!

@rasyidf
Copy link
Contributor

rasyidf commented Aug 12, 2022

I'm just starting this task, hope I can finish it this weekend.

🎉 congratulations!!!

thank you :D

@rasyidf
Copy link
Contributor

rasyidf commented Aug 12, 2022

  • I'm still don't really understand how Image being managed.
  • do I have to add deleteById in the Image Repository? or Is it Already Exist?
  • Is hotkey for deletion is necessary to be added?

@SkalskiP
Copy link
Owner

I'm still don't really understand how Image being managed.

Image management is divided into two parts: ImageRepository and LabelsState. LabelsState.imagesData field stores ImageData objects. Each ImageData contains fields like id, File as well as lots of label data. ImageRepository is essentially a map matching image id from ImageData with actual HTMLImageElement.

File is the object that I get from the browser when the user loads images. It contains (among other things) the path to the image. HTMLImageElement is the actual image data. File is light and HTMLImageElement is heavy.

I decided that I need to separate HTMLImageElement from the rest of the image data. It's because I store most of the data in Redux, and Redux requires you to constantly return a new state, which is a copy of an old state with small changes. I didn't want to constantly juggle large HTMLImageElement objects.

When the user uploads an image, the browser is passing us the File object. We take it and wrap it into ImageData and store it in Redux. When the user wants to display an image represented by this File we actually load it and at the same time store loaded HTMLImageElement in ImageRepository.

I hope it is more clear now.

do I have to add deleteById in the Image Repository? or Is it Already Exist?

I would do two things. First: add deleteById to ImageRepository. That would be responsible for removing HTMLImageElement object. We need to make sure it is actually gone from memory. Second: add deleteById action to ImageActions, that would be responsible for updating Redux.

Is hotkey for deletion is necessary to be added?

No I don't plan to add hotkey for that functionality for now.

@rasyidf
Copy link
Contributor

rasyidf commented Aug 13, 2022

Whoa, thanks for the explanation, I'll try to implement it soon.

@rasyidf
Copy link
Contributor

rasyidf commented Aug 13, 2022

Actually, I'm new with Redux, so it would take some time.

@SkalskiP
Copy link
Owner

No worries I understand 🤯! Just let me know if you will have any questions. I’ll try to be as responsive as possible.

@rasyidf rasyidf mentioned this issue Apr 6, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ms-beta Part of make-sense beta efforts new-feature New feature or request
Projects
Development

No branches or pull requests

4 participants