Skip to content

Use GetUniqueCombinations_new instead of GetUniqueCombinations #8214

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.

Sign up for GitHub

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

Merged
greglandrum merged 5 commits into from
Feb 6, 2025
Merged

Use GetUniqueCombinations_new instead of GetUniqueCombinations #8214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2ccef13
Remove GetUniqueCombinations, use instead GetUniqueCombinations_new f…
bertiewooster Jan 26, 2025
9ab869f
Remove unused and uncalled parameter `which`
bertiewooster Jan 26, 2025
f1a58ed
Update tests for GetUniqueCombinations to be in numerical order. Remo…
bertiewooster Jan 28, 2025
bc6b07c
Create _compareCombinationsOrderUnimportant and use for testGetUnique…
bertiewooster Jan 28, 2025
85015f4
Replace call to GetUniqueCombinations_new with GetUniqueCombinations …
bertiewooster Jan 28, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions ReleaseNotes.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
# Release_2025.03.1
(Changes relative to Release_2024.09.1)
## Backwards incompatible changes
- The order of combinations returned by Chem.Pharm2D.Utils.GetUniqueCombinations has changed to be in numerical order. The combinations themselves are unchanged.

## Acknowledgements
(Note: I'm no longer attempting to manually curate names. If you would like to
Expand Down
34 changes: 15 additions & 19 deletions rdkit/Chem/Pharm2D/UnitTestUtils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,48 +123,44 @@ def _compareCombinations(self, actual, expected):
print(actual)
self.assertEqual(actual, expected, 'Combinations are not in the same order')

def testGetUniqueCombinations(self):
x = [[(1, (11, )), (1, (12, )), (2, (31, ))], [(1, (11, )), (1, (12, )), (2, (32, ))],
[(1, (11, )), (1, (13, )), (2, (31, ))], [(1, (11, )), (1, (13, )), (2, (32, ))],
[(1, (11, )), (1, (14, )), (2, (31, ))], [(1, (11, )), (1, (14, )), (2, (32, ))],
[(1, (12, )), (1, (13, )), (2, (31, ))], [(1, (12, )), (1, (13, )), (2, (32, ))],
[(1, (12, )), (1, (14, )), (2, (31, ))], [(1, (12, )), (1, (14, )), (2, (32, ))],
[(1, (13, )), (1, (14, )), (2, (31, ))], [(1, (13, )), (1, (14, )), (2, (32, ))]]
def _compareCombinationsOrderUnimportant(self, actual, expected):
self.assertEqual(sorted(actual), sorted(expected), 'Combinations are different')

def testGetUniqueCombinations(self):
# yapf: disable
GetUniqueCombinations = Utils.GetUniqueCombinations

choices = [[(11,), (12,)], [(21,), (22,)], [(31,), (32,)]]
classes = [1, 2, 3]
self._compareCombinations(
self._compareCombinationsOrderUnimportant(
GetUniqueCombinations(choices, classes),
[[(1, (11,)), (2, (21,)), (3, (31,))], [(1, (11,)), (2, (21,)), (3, (32,))],
[(1, (11,)), (2, (22,)), (3, (31,))], [(1, (11,)), (2, (22,)), (3, (32,))],
[(1, (12,)), (2, (21,)), (3, (31,))], [(1, (12,)), (2, (21,)), (3, (32,))],
[(1, (12,)), (2, (22,)), (3, (31,))], [(1, (12,)), (2, (22,)), (3, (32,))]])

classes = [1, 1, 2]
self._compareCombinations(
self._compareCombinationsOrderUnimportant(
GetUniqueCombinations(choices, classes),
[[(1, (11,)), (1, (21,)), (2, (31,))], [(1, (11,)), (1, (21,)), (2, (32,))],
[(1, (11,)), (1, (22,)), (2, (31,))], [(1, (11,)), (1, (22,)), (2, (32,))],
[(1, (12,)), (1, (21,)), (2, (31,))], [(1, (12,)), (1, (21,)), (2, (32,))],
[(1, (12,)), (1, (22,)), (2, (31,))], [(1, (12,)), (1, (22,)), (2, (32,))]])

choices = [[(11,), (12,)], [(11,), (12,)], [(31,), (32,)]]
self._compareCombinations(
self._compareCombinationsOrderUnimportant(
GetUniqueCombinations(choices, classes),
[[(1, (11,)), (1, (12,)), (2, (31,))], [(1, (11,)), (1, (12,)), (2, (32,))]])

choices = [[(11,), (12,)], [(11,), (12,), (13,)], [(31,), (32,)]]
self._compareCombinations(
self._compareCombinationsOrderUnimportant(
GetUniqueCombinations(choices, classes),
[[(1, (11,)), (1, (12,)), (2, (31,))], [(1, (11,)), (1, (12,)), (2, (32,))],
[(1, (11,)), (1, (13,)), (2, (31,))], [(1, (11,)), (1, (13,)), (2, (32,))],
[(1, (12,)), (1, (13,)), (2, (31,))], [(1, (12,)), (1, (13,)), (2, (32,))]])

choices = [[(11,), (12,), (14,)], [(11,), (12,), (13,)], [(31,), (32,)]]
self._compareCombinations(
self._compareCombinationsOrderUnimportant(
GetUniqueCombinations(choices, classes),
[[(1, (11,)), (1, (12,)), (2, (31,))], [(1, (11,)), (1, (12,)), (2, (32,))],
[(1, (11,)), (1, (13,)), (2, (31,))], [(1, (11,)), (1, (13,)), (2, (32,))],
Expand All @@ -174,25 +170,25 @@ def testGetUniqueCombinations(self):
[(1, (13,)), (1, (14,)), (2, (31,))], [(1, (13,)), (1, (14,)), (2, (32,))]])

choices = [[(11, 111,), (12,), (14,)], [(11, 111,), (12,), (13,)], [(31,), (32,)]]
self._compareCombinations(
self._compareCombinationsOrderUnimportant(
GetUniqueCombinations(choices, classes),
[[(1, (11, 111)), (1, (12,)), (2, (31,))], [(1, (11, 111)), (1, (12,)), (2, (32,))],
[(1, (11, 111)), (1, (13,)), (2, (31,))], [(1, (11, 111)), (1, (13,)), (2, (32,))],
[(1, (12,)), (1, (13,)), (2, (31,))], [(1, (12,)), (1, (13,)), (2, (32,))],
[(1, (11, 111)), (1, (14,)), (2, (31,))], [(1, (11, 111)), (1, (14,)), (2, (32,))],
[(1, (12,)), (1, (13,)), (2, (31,))], [(1, (12,)), (1, (13,)), (2, (32,))],
[(1, (12,)), (1, (14,)), (2, (31,))], [(1, (12,)), (1, (14,)), (2, (32,))],
[(1, (13,)), (1, (14,)), (2, (31,))], [(1, (13,)), (1, (14,)), (2, (32,))]])

choices = [[(11,), (12,), ], [(11,), (13,), ], [(11,), (14,), ]]
classes = [1, 1, 1]
self._compareCombinations(
self._compareCombinationsOrderUnimportant(
GetUniqueCombinations(choices, classes),
[[(1, (11,)), (1, (13,)), (1, (14,))], [(1, (11,)), (1, (12,)), (1, (14,))],
[(1, (11,)), (1, (12,)), (1, (13,))], [(1, (12,)), (1, (13,)), (1, (14,))]])
[[(1, (11,)), (1, (12,)), (1, (13,))], [(1, (11,)), (1, (12,)), (1, (14,))],
[(1, (11,)), (1, (13,)), (1, (14,))], [(1, (12,)), (1, (13,)), (1, (14,))]])

choices = [[(0,), (4,)], [(0,)]]
classes = [0, 1]
self._compareCombinations(GetUniqueCombinations(choices, classes), [[(0, (4,)), (1, (0,))]])
self._compareCombinationsOrderUnimportant(GetUniqueCombinations(choices, classes), [[(0, (4,)), (1, (0,))]])

choices = [[], [], []]
classes = [0, 1, 1]
Expand All @@ -209,7 +205,7 @@ def testGetUniqueCombinations(self):
choices = [[(4,), (6,), (7,), (10,)], [(2,), (3,), (5,), (13,), (14,)],
[(2,), (3,), (5,), (13,), (14,)]]
classes = [0, 1, 1]
self._compareCombinations(
self._compareCombinationsOrderUnimportant(
GetUniqueCombinations(choices, classes),
[[(0, (4,)), (1, (2,)), (1, (3,))], [(0, (4,)), (1, (2,)), (1, (5,))],
[(0, (4,)), (1, (2,)), (1, (13,))], [(0, (4,)), (1, (2,)), (1, (14,))],
Expand Down
28 changes: 2 additions & 26 deletions rdkit/Chem/Pharm2D/Utils.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -321,36 +321,11 @@ def GetAllCombinations(choices, noDups=1, which=0):
return res


def GetUniqueCombinations(choices, classes, which=0):
def GetUniqueCombinations(choices, classes):
""" Does the combinatorial explosion of the possible combinations
of the elements of _choices_.

"""
# print(choices, classes)
assert len(choices) == len(classes)
if which >= len(choices):
return []
if which == len(choices) - 1:
return [[(classes[which], x)] for x in choices[which]]

res = []
tmp = GetUniqueCombinations(choices, classes, which=which + 1)
for thing in choices[which]:
for other in tmp:
if not any(x[1] == thing for x in other):
newL = [(classes[which], thing)] + other
newL.sort()
if newL not in res:
res.append(newL)
return res


def GetUniqueCombinations_new(choices, classes, which=0):
""" Does the combinatorial explosion of the possible combinations
of the elements of _choices_.

"""
# print(choices, classes)
assert len(choices) == len(classes)
combos = set()
for choice in itertools.product(*choices):
Expand All @@ -360,6 +335,7 @@ def GetUniqueCombinations_new(choices, classes, which=0):
combos.add(tuple(sorted((cls, ch) for cls, ch in zip(classes, choice))))
return [list(combo) for combo in sorted(combos)]

GetUniqueCombinations_new = GetUniqueCombinations

def UniquifyCombinations(combos):
""" uniquifies the combinations in the argument
Expand Down