Add topological sort option to list* commands #62

ThomasBinsfeld · 2023-10-30T15:04:34Z

Add a new --sort option on the list-depend command allowing to apply a topological sorting to the result.
https://docs.python.org/3/library/graphlib.html

src/manifestoo/commands/list_depends.py

src/manifestoo/main.py

sbidoul · 2023-10-30T16:15:21Z

I see graphlib is python 3.9+, but that is ok, we can simply error out if the topological sort is selected by the user and sys.version_info < (3,9).

src/manifestoo/addon_sorter.py

src/manifestoo/main.py

sbidoul · 2023-10-30T16:26:56Z

This is taking shape. Can you add tests for the sorters and update the documentation too?

src/manifestoo/commands/list_depends.py

src/manifestoo/addon_sorter.py

sbidoul · 2023-10-30T17:22:03Z

Can you also

add a news/62.feature with a short changelog entry
add the sort option to the list and list-codepends commands

src/manifestoo/addon_sorter.py

src/manifestoo/commands/list_codepends.py

tests/test_cmd_list_depends.py

src/manifestoo/addon_sorter.py

sbidoul · 2023-10-31T13:56:14Z

src/manifestoo/addon_sorter.py

+                result_dict[addon_name] = set(
+                    [
+                        depend
+                        for depend in addon.manifest.depends
+                        if depend in addons_selection
+                    ]
+                )


Suggested change

result_dict[addon_name] = set(

[

depend

for depend in addon.manifest.depends

if depend in addons_selection

]

)

result_dict[addon_name] = set(

depend

for depend in addon.manifest.depends

if depend in addons_selection

)

sbidoul · 2023-10-31T13:57:50Z

src/manifestoo/commands/list_codepends.py

    result: Set[str] = set(addons_selection) if include_selected else set()
    codeps = direct_codependencies(addons_selection, addons_set, result)
    result |= codeps
    while transitive and codeps:
        codeps = direct_codependencies(codeps, addons_set, result)
        result |= codeps
-    return result if include_selected else result - addons_selection
+    res = result if include_selected else result - set(addons_selection)


Suggested change

res = result if include_selected else result - set(addons_selection)

res = result if include_selected else result - addons_selection

This change was not necessary?

sbidoul · 2023-10-31T13:58:07Z

src/manifestoo/commands/list_codepends.py

    result: Set[str] = set(addons_selection) if include_selected else set()
    codeps = direct_codependencies(addons_selection, addons_set, result)
    result |= codeps
    while transitive and codeps:
        codeps = direct_codependencies(codeps, addons_set, result)
        result |= codeps
-    return result if include_selected else result - addons_selection
+    res = result if include_selected else result - set(addons_selection)
+    return set(addon_sorter.sort(res, addons_set))


Suggested change

return set(addon_sorter.sort(res, addons_set))

return addon_sorter.sort(res, addons_set)

Set would defeat the sorting.

sbidoul · 2023-10-31T14:05:42Z

src/manifestoo/main.py

+        "--sorting",
+        "--sort",


Suggested change

"--sorting",

"--sort",

"--sort",

--sorting sounds a bit strange to me. Not sure.

sbidoul · 2023-10-31T14:08:18Z

src/manifestoo/main.py

+        "--sorting",
+        "--sort",
+        help=(
+            "Choice between 'alphabetical' and 'topological'."


Suggested change

"Choice between 'alphabetical' and 'topological'."

"Choice between 'alphabetical' and 'topological'. "

Also for the 2 others

Using https://docs.python.org/3/library/graphlib.html