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

Fix refs appearing in dupes-only view #910

Merged
merged 1 commit into from
Jun 23, 2021
Merged

Conversation

glubsy
Copy link
Contributor

@glubsy glubsy commented Jun 22, 2021

  • Some refs appeared in the dupes-only view after a re-prioritization was done a second time.
  • It seems the core.Results.__dupes list was not properly updated whenever core.app.Dupeguru.reprioritize_groups() -> core.Results.sort_dupes() was called.
    When a re-prioritization is done, some refs became dupe, and some dupes became ref in their place. So we need to update the new state of the internal list of dupes kept by the Results object, instead of relying on the outdated cached one.
  • Fix Dupes Only view is not updated after Re-Prioritize Results #757.

* Some refs appeared in the dupes-only view after a re-prioritization was done a second time.
* It seems the core.Results.__dupes list was not properly updated whenever core.app.Dupeguru.reprioritize_groups() -> core.Results.sort_dupes() was called.
When a re-prioritization is done, some refs became dupe, and some dupes became ref in their place. So we need to update the new state of the internal list of dupes kept by the Results object, instead of relying on the outdated cached one.
* Fix arsenetar#757.
@arsenetar
Copy link
Owner

Issue with this change seems to come from the sort_dupes() and __get_dupe_list() calling each other recursively, due to __get_dupe_list no longer having the blocking conditional on line 97.

@arsenetar
Copy link
Owner

I just did a quick test splitting out sort_dupes() into two functions as below:

    def sort_dupes(self, key, asc=True, delta=False):
        """Sort :attr:`dupes` according to ``key``.

        :param str key: key attribute name to sort with.
        :param bool asc: If false, sorting is reversed.
        :param bool delta: If true, sorting occurs using :ref:`delta values <deltavalues>`.
        """
        if not self.__dupes:
            self.__get_dupe_list()
        self.__sort_dupes(key, asc, delta)

    def __sort_dupes(self, key, asc, delta):
        keyfunc = lambda d: self.app._get_dupe_sort_key(
            d, lambda: self.get_group_of_duplicate(d), key, delta
        )
        self.__dupes.sort(key=keyfunc, reverse=not asc)
        self.__dupes_sort_descriptor = (key, asc, delta)

I changed the __get_dupe_list() to call the __sort_dupes() function directly. With that it seems only the "cached results list" test fails.

@glubsy glubsy mentioned this pull request Jun 23, 2021
@glubsy
Copy link
Contributor Author

glubsy commented Jun 23, 2021

Ah well, I did it slightly differently and the caching works just like before. The cache only gets updated whenever a group has changed.

@arsenetar
Copy link
Owner

Yeah I was just trying to verify that was the primary issue, I think the way you fixed it is better and keeps the "cached results" working.

@glubsy glubsy deleted the 757_fix branch June 23, 2021 03:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dupes Only view is not updated after Re-Prioritize Results
2 participants