Remove redundant dependency checks #556
Conversation
|
My editor removed all the trailing whitespace from the modified file. If this is undesired, I can undo. |
|
@jacobperron please avoid them in the future. I'll review this one wi the |
src/rosdep2/installers.py
Outdated
| return [x for x in resolved if x not in self.detect_fn(resolved)] | ||
| detected = set(self.detect_fn(resolved)) | ||
| seen = set() | ||
| seen_add = seen.add |
There was a problem hiding this comment.
Why do you need to do this? It's the same number of characters.
There was a problem hiding this comment.
For execution speed. Reference.
Perhaps it is overkill.
There was a problem hiding this comment.
It's fine, I just wasn't aware of why. You might want a comment, otherwise someone might just remove it as redundant.
src/rosdep2/installers.py
Outdated
| detected = set(self.detect_fn(resolved)) | ||
| seen = set() | ||
| seen_add = seen.add | ||
| return [x for x in resolved if (x not in detected) and (x not in seen or seen_add(x))] |
There was a problem hiding this comment.
What is the seen set for? Would this not be simpler:
detected = self.detect_fn(resolved)
return [x for x in resolved if x not in detected]There was a problem hiding this comment.
Yes, it would appear I overthought the problem.
|
Alright, simplified and removed the trailing space edits 👍 |
Fix for #555.
This should maintain dependency order (desired from #545) while eliminating duplicate dependencies from the returned list.
@LWisteria, does this negatively impact you?