Implement *-discarded actions #146

vladox · 2020-12-29T00:16:03Z

Preliminary checks

I am running the latest version
I have read the Code of Conduct
I have checked there is not other Issues or Pull Requests covering the same topic to open

Describe the bug

When running mdedup -a delete-discarded -s select-one ./.MyMailDirFolder I get the follwoing result:

NotImplementedError: delete-discarded action not implemented yet.

To reproduce

Steps to reproduce the behavior:

The full mdedup -a delete-discarded -s select-one ./.MyMailDirFolder CLI invocation you used.
```
$ mdedup -a delete-discarded -s select-one ./.MyMailDirFolder
```
The data set leading to the bug.
Try to produce here the minimal subset of mails leading to the bug, and add copies of those mails (eventually censored).
This effort will help maintainers add this particular edge-case to the set of unittests to prevent future regressions.
You can reduce down the issue to a particular deduplicate subset by using the --hash-only parameter.

Expected behavior

Duplicated emails being deleted

Is this an expected behaviour?

The text was updated successfully, but these errors were encountered:

vincentbernat · 2021-01-05T15:41:50Z

I also ran into this bug. For some reason, only the *-selected actions are implemented. So, in many cases, you can use -a delete-selected (and maybe repeat several times if you get several duplicates).

Edit: very bad idea. Non-duplicated mails will be selected too...

leggewie · 2021-01-14T03:05:50Z

same is true for move-discarded

andy-shev · 2021-01-20T11:08:43Z

So the core functionality of the tool is not implemented... Can we prioritize this bug (which actually makes tool useless) to be fixed sooner than later?

kdeldycke · 2021-01-20T15:12:34Z

@vincentbernat is right. All *-discarded actions are not implemented yet.

In the mean time, you can indeed emulate the effect of -a delete-discarded -s select-one with -a delete-selected -s select-all-but-one. I.e. by reversing the logic.

The thing is all *-discarded actions are syntactic sugar. They should implement the reverse of *-selected actions at:

mail-deduplicate/mail_deduplicate/action.py

Lines 82 to 91 in f423b41

    
           ACTIONS = FrozenDict( 
        
               { 
        
                   COPY_SELECTED: copy_selected, 
        
                   COPY_DISCARDED: None, 
        
                   MOVE_SELECTED: move_selected, 
        
                   MOVE_DISCARDED: None, 
        
                   DELETE_SELECTED: delete_selected, 
        
                   DELETE_DISCARDED: None, 
        
               } 
        
           )

Same for strategies by the way. They all have their own aliases to allow users to better map their own worldview to the operational logic. See:

mail-deduplicate/mail_deduplicate/strategy.py

Lines 206 to 224 in f423b41

    
           # Groups strategy aliases and their definitions. Aliases are great useability 
        
           # features as it helps users to better reason about the selection operators 
        
           # dependening on their mental models. 
        
           STRATEGY_ALIASES = frozenset( 
        
               [ 
        
                   (SELECT_NEWEST, DISCARD_OLDER), 
        
                   (SELECT_NEWER, DISCARD_OLDEST), 
        
                   (SELECT_OLDEST, DISCARD_NEWER), 
        
                   (SELECT_OLDER, DISCARD_NEWEST), 
        
                   (SELECT_BIGGEST, DISCARD_SMALLER), 
        
                   (SELECT_BIGGER, DISCARD_SMALLEST), 
        
                   (SELECT_SMALLEST, DISCARD_BIGGER), 
        
                   (SELECT_SMALLER, DISCARD_BIGGEST), 
        
                   (SELECT_NON_MATCHING_PATH, DISCARD_MATCHING_PATH), 
        
                   (SELECT_MATCHING_PATH, DISCARD_NON_MATCHING_PATH), 
        
                   (SELECT_ALL_BUT_ONE, DISCARD_ONE), 
        
                   (SELECT_ONE, DISCARD_ALL_BUT_ONE), 
        
               ] 
        
           )

The problem is at the selection process step, right before we perform the action:

mail-deduplicate/mail_deduplicate/deduplicate.py

Line 277 in f423b41

selected = apply_strategy(self.conf.strategy, self)

Applying the strategy return the list of selected mails, not the one that were discarded. See how each implemented strategies only returns the subset of the duplicate pool:

mail-deduplicate/mail_deduplicate/strategy.py

Lines 37 to 39 in f423b41

    
           return { 
        
               mail for mail in duplicates.pool if mail.timestamp < duplicates.newest_timestamp 
        
           }

mail-deduplicate/mail_deduplicate/strategy.py

Lines 52 to 56 in f423b41

    
           return { 
        
               mail 
        
               for mail in duplicates.pool 
        
               if mail.timestamp == duplicates.oldest_timestamp 
        
           }

and so on...

By the time we need to perform the action, we only have a subset of the initial duplicate pool. We do not have the list of those that were discarded.

This is the limit of the code architecture inherited from the initial dumb script I wrote 10 years ago. It was an early optimization to reduce the memory footprint. Given that context, it will be hard to easily implement the action with the current code structure.

I propose to first tackle #87, i.e. keep a cache of canonical hashes used to ID each mail. That way we'll be in a position to only deal with sets of hashes in our selection/action phases instead of parsed mail objects. This will bring much cleaner and flexible code to implement the missing actions.

andy-shev · 2021-01-20T17:14:10Z

@kdeldycke
The problem is that single mails are got removed by your proposal. So, it's not equivalent substitution, please revert "bug" tag and prioritize the fix.

mdedup -n -i maildir -s select-all-but-one -a delete-selected -C 32 .

● Phase #4 - Report and statistics
╒════════════╤══════════╤══════════════════════════════════════════════════════════════╕
│ Mails      │   Metric │ Description                                                  │
╞════════════╪══════════╪══════════════════════════════════════════════════════════════╡
│ Found      │   221373 │ Total number of mails encountered from all mail sources.     │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Rejected   │     1067 │ Number of mails individuality rejected because they were     │
│            │          │ unparseable or did not had enough metadata to compute        │
│            │          │ hashes.                                                      │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Retained   │   220306 │ Number of valid mails parsed and retained for deduplication. │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Hashes     │   165621 │ Number of unique hashes.                                     │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Unique     │   121943 │ Number of unique mails (which where automaticcaly added to   │
│            │          │ selection).                                                  │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Duplicates │    98363 │ Number of duplicate mails (sum of mails in all duplicate     │
│            │          │ sets with at least 2 mails).                                 │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Skipped    │     8644 │ Number of mails ignored in the selection phase because the   │
│            │          │ whole set they belongs to was skipped.                       │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Discarded  │    40268 │ Number of mails discarded from the final selection.          │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Selected   │   171394 │ Number of mails kept in the final selection on which the     │
│            │          │ action will be performed.                                    │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Copied     │        0 │ Number of mails copied from their original mailbox to        │
│            │          │ another.                                                     │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Moved      │        0 │ Number of mails moved from their original mailbox to         │
│            │          │ another.                                                     │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Deleted    │   171394 │ Number of mails deleted from their mailbox in-place.         │
╘════════════╧══════════╧══════════════════════════════════════════════════════════════╛
╒════════════════════╤══════════╤════════════════════════════════════════════════════════════╕
│ Duplicate sets     │   Metric │ Description                                                │
╞════════════════════╪══════════╪════════════════════════════════════════════════════════════╡
│ Total              │   165621 │ Total number of duplicate sets.                            │
├────────────────────┼──────────┼────────────────────────────────────────────────────────────┤
│ Single             │   121943 │ Total number of sets containing a single mail and did not  │
│                    │          │ had to have a strategy applied to. They were automatticaly │
│                    │          │ kept in the final selection.                               │
├────────────────────┼──────────┼────────────────────────────────────────────────────────────┤
│ Skipped - Encoding │        0 │ Number of sets skipped from the selection process because  │
│                    │          │ they had encoding issues.                                  │
├────────────────────┼──────────┼────────────────────────────────────────────────────────────┤
│ Skipped - Size     │      510 │ Number of sets skipped from the selection process because  │
│                    │          │ they were too disimilar in size.                           │
├────────────────────┼──────────┼────────────────────────────────────────────────────────────┤
│ Skipeed - Content  │        0 │ Number of sets skipped from the selection process because  │
│                    │          │ they were too disimilar in content.                        │
├────────────────────┼──────────┼────────────────────────────────────────────────────────────┤
│ Skipped - Strategy │        0 │ Number of sets skipped from the selection process because  │
│                    │          │ the strategy could not be applied.                         │
├────────────────────┼──────────┼────────────────────────────────────────────────────────────┤
│ Deduplicated       │    40268 │ Number of valid sets on which the selection strategy was   │
│                    │          │ successfully applied.                                      │
╘════════════════════╧══════════╧════════════════════════════════════════════════════════════╛

nutria007 · 2021-01-20T18:02:35Z

I've managed to do the inteded action by making it skip unique emails:

https://github.com/nutria007/mail-deduplicate/blob/872ec4a10dd391c9ad1650e7317b75dd52d24abe/mail_deduplicate/deduplicate.py#L404-L422

mail_deduplicate/mail_deduplicate/deduplicate.py
Lines 404 to 422

            # Unique mails are always selected. No need to mobilize the whole
            # DuplicateSet machinery.
            if mail_count == 1:
                self.stats["mail_unique"] += 1
                self.stats["set_single"] += 1
                if self.conf.action == "delete-selected":
                    logger.debug("Skipped deletion of unique mail.")
                    self.stats["mail_skipped"] += 1
                else:
                    logger.debug("Add unique message to selection.")
                    self.stats["mail_selected"] += 1
                    candidates = mail_set
            # We need to resort to a selection strategy to discriminate mails
            # within the set.
            else:
                duplicates = DuplicateSet(hash_key, mail_set, self.conf)
                candidates = duplicates.select_candidates()
                # Merge duplicate set's stats to global stats.
                self.stats += duplicates.stats

andy-shev · 2021-01-20T18:37:06Z

I've managed to do the inteded action by making it skip unique emails:

mail_deduplicate/mail_deduplicate/deduplicate.py Lines 404 to 422

Thanks, it;s exact place where I'm coding right now for myself, but on top I introduced a new option.

dschrempf · 2021-02-09T15:30:02Z

Any progress on this? select-newer also doesn't work as expected. Single mails without duplicates are still selected.

│ Mails      │   Metric │ Description                                                  │
╞════════════╪══════════╪══════════════════════════════════════════════════════════════╡
│ Found      │     1060 │ Total number of mails encountered from all mail sources.     │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Rejected   │        0 │ Number of mails rejected individually because they were      │
│            │          │ unparseable or did not have enough metadata to compute       │
│            │          │ hashes.                                                      │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Retained   │     1060 │ Number of valid mails parsed and retained for deduplication. │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Hashes     │     1060 │ Number of unique hashes.                                     │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Unique     │     1060 │ Number of unique mails (which where automatically added to   │
│            │          │ selection).                                                  │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Duplicates │        0 │ Number of duplicate mails (sum of mails in all duplicate     │
│            │          │ sets with at least 2 mails).                                 │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Skipped    │        0 │ Number of mails ignored in the selection phase because the   │
│            │          │ whole set they belong to was skipped.                        │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Discarded  │        0 │ Number of mails discarded from the final selection.          │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Selected   │     1060 │ Number of mails kept in the final selection on which the     │
│            │          │ action will be performed.                                    │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Copied     │        0 │ Number of mails copied from their original mailbox to        │
│            │          │ another.                                                     │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Moved      │        0 │ Number of mails moved from their original mailbox to         │
│            │          │ another.                                                     │
├────────────┼──────────┼──────────────────────────────────────────────────────────────┤
│ Deleted    │     1060 │ Number of mails deleted from their mailbox in-place.         │

Command line was

 mdedup -s select-newer -a delete-selected -n -t ctime maildir

leggewie · 2021-02-12T20:08:57Z

@dschrempf Can you please open a new ticket regarding select-newer misbehaving?

djwf · 2021-03-04T02:43:58Z

FYI: just added an issue and PR regarding the option to only act on duplicates (#203 and #204, respectively)

kdeldycke · 2021-09-12T10:51:40Z

The missing *-discarded actions have been implemented in #290 and are now available upstream.

github-actions · 2021-12-13T08:51:17Z

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

vladox added the bug label Dec 29, 2020

vladox assigned kdeldycke Dec 29, 2020

vladox changed the title ~~\U0001F41B delete-discarded action not implemented yet~~ delete-discarded action not implemented yet Dec 29, 2020

kdeldycke added enhancement labels Jan 20, 2021

kdeldycke changed the title ~~delete-discarded action not implemented yet~~ All *-discarded action not implemented yet Jan 20, 2021

kdeldycke removed bug labels Jan 20, 2021

kdeldycke changed the title ~~All *-discarded action not implemented yet~~ Implement *-discarded actions Jan 20, 2021

djwf mentioned this issue Mar 4, 2021

Add option to ignore single messages when performing any actions #203

Closed

kdeldycke mentioned this issue Mar 15, 2021

Add option to only act on duplicate messages #204

Closed

8 tasks

NCenerar mentioned this issue Sep 12, 2021

✨ Implements missing discard actions #290

Merged

8 tasks

kdeldycke closed this as completed in #290 Sep 12, 2021

github-actions bot locked as resolved and limited conversation to collaborators Dec 13, 2021

kdeldycke added ✨ enhancement Improvement or change to an existing feature and removed enhancement labels Nov 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement *-discarded actions #146

Implement *-discarded actions #146

vladox commented Dec 29, 2020 •

edited by kdeldycke

vincentbernat commented Jan 5, 2021 •

edited

leggewie commented Jan 14, 2021

andy-shev commented Jan 20, 2021

kdeldycke commented Jan 20, 2021

andy-shev commented Jan 20, 2021 •

edited

nutria007 commented Jan 20, 2021 •

edited by kdeldycke

andy-shev commented Jan 20, 2021

dschrempf commented Feb 9, 2021

leggewie commented Feb 12, 2021

djwf commented Mar 4, 2021

kdeldycke commented Sep 12, 2021

github-actions bot commented Dec 13, 2021

Implement *-discarded actions #146

Implement *-discarded actions #146

Comments

vladox commented Dec 29, 2020 • edited by kdeldycke

Preliminary checks

Describe the bug

To reproduce

Expected behavior

vincentbernat commented Jan 5, 2021 • edited

leggewie commented Jan 14, 2021

andy-shev commented Jan 20, 2021

kdeldycke commented Jan 20, 2021

andy-shev commented Jan 20, 2021 • edited

nutria007 commented Jan 20, 2021 • edited by kdeldycke

andy-shev commented Jan 20, 2021

dschrempf commented Feb 9, 2021

leggewie commented Feb 12, 2021

djwf commented Mar 4, 2021

kdeldycke commented Sep 12, 2021

github-actions bot commented Dec 13, 2021

vladox commented Dec 29, 2020 •

edited by kdeldycke

vincentbernat commented Jan 5, 2021 •

edited

andy-shev commented Jan 20, 2021 •

edited

nutria007 commented Jan 20, 2021 •

edited by kdeldycke