Auto cleanup memmap scores #1086
Conversation
| @@ -175,7 +174,6 @@ def partition( | |||
| clusters = self.cluster(pair_scores, threshold) | |||
| clusters = self._add_singletons(data, clusters) | |||
| clusters = list(clusters) | |||
There was a problem hiding this comment.
Now that we have auto-cleanup, perhaps we don't want to cast this to a list? I think then this entire method would be out-of-core, right?
Same for casting to list in join()
There was a problem hiding this comment.
that's right. i think we we should put in a warning for a few releases though because some folks are likely depending on it being a list instead of a generator
There was a problem hiding this comment.
do we want to do what Gazeteer.search() does with the generator: bool parameter?
def score(.... generator: bool = False):
...
if not generator:
warn("this deprecated")
return list(scores)
else:
yield from scores
dedupe/core.py
Outdated
| @@ -176,9 +177,28 @@ def scoreDuplicates( | |||
| else: | |||
| scored_pairs = numpy.array([], dtype=dtype) | |||
|
|
|||
| # See https://docs.python.org/3/library/weakref.html#comparing-finalizers-with-del-methods | |||
| scored_pairs.remove = weakref.finalize(scored_pairs, _cleanup_scores, scored_pairs) | |||
| scored_pairs.removed = property(_is_removed) | |||
There was a problem hiding this comment.
IDK if including the removed property is good, probably could live without it.
There was a problem hiding this comment.
if i understand correctly, it's never going to be anything except false in reachable code? because it would only set to true when scored_paris is garbage collected?
There was a problem hiding this comment.
Someone could call pairs.remove() manually to do the finalization, and then it would be True. But you're correct, we as the library rely on garbage collection so we never take advantage of the .removed property. We could remove it, always easier to add later than to have to continue supporting it forever...
NickCrews
force-pushed
the
auto-cleanup
branch
from
a61dab1
to
815cde5
Compare
It wasn't stated anywhere in the docs that you have to cleanup the possibly-a-tempfile, so people might not have been doing it. And, I thought this was better than the alternatives from https://stackoverflow.com/questions/865115/how-do-i-correctly-clean-up-a-python-object
Now you can #type: ignore[specific-code]
NickCrews
force-pushed
the
auto-cleanup
branch
from
815cde5
to
133d744
Compare
| @@ -176,9 +177,29 @@ def scoreDuplicates( | |||
| else: | |||
| scored_pairs = numpy.array([], dtype=dtype) | |||
|
|
|||
| # Monkeypatch in these extra methods and attributes. | |||
There was a problem hiding this comment.
Yeah seriously! The first time I've used it too.
|
wow! i've long wanted to do something like this but del was always so finicky. a super nice approach. can we get a test for this: something like |
|
Yes the test seems important, I can look into it |
Users don't need to worry about cleaning up the memmpa file now