Fix refs appearing in dupes-only view#910
Merged
Merged
Conversation
* 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.
Merged
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. |
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. |
Merged
Contributor
Author
|
Ah well, I did it slightly differently and the caching works just like before. The cache only gets updated whenever a group has changed. |
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.