Repository: conda-incubator/grayskull (python)

Issue:
Maintaining dependency order
**Is your feature request related to a problem? Please describe.**

When looking at diffs of dependencies, sometimes they are hard to reason about due to reordering of the dependencies. Additionally some dependencies (like `python` tend to get listed first to make it easier to see they are there (and not forgotten) with any relevant version constraints.

**Describe the solution you'd like**

It would help if grayskull was able to maintain the dependency order as much as possible. This would keep diffs more compact and making it easier to catch any errors (like C/C++ dependencies that got dropped).

**Describe alternatives you've considered**

One alternative instead of following the recipe order would be to follow the `setup.py` order. As maintainers are often comparing the two, this actually makes that comparison easier. Though if the recipe is doing something different, this might be off for the first PR still

Feel free to suggest alternatives.

**Additional context**

Have made and looked at a few PRs with Grayskull recently and this behavior has resulted in some confusion.

Pull request description:
Maintain dependency order
Currently dependencies are alphabetically sorted (with the exception of Python which goes at the top). This can be confusing when comparing Grayskull's output to the upstream repository. This was previously requested in #259.

In general I find alphabetization good since it lowers entropy. But as far as Grayskull is concerned, it feels to me like alphabetization would be better done upstream. But I could understand people wanting to preserve the alphabetization behavior.

In case we decide against alphabetization, I hope that we could still merge all but the last commit which flips the switch.

Closes #259 


Patch:
diff --git a/grayskull/strategy/pypi.py b/grayskull/strategy/pypi.py
index 562bd2f..298f80a 100644
--- a/grayskull/strategy/pypi.py
+++ b/grayskull/strategy/pypi.py
@@ -543,8 +543,8 @@ def extract_requirements(metadata: dict, config, recipe) -> Dict[str, List[str]]
     return result
 
 
-def sort_reqs(reqs: Iterable[str]) -> List[str]:
-    """Sort requirements. Put python first, then sort alphabetically."""
+def sort_reqs(reqs: Iterable[str], alphabetize: bool = False) -> List[str]:
+    """Sort requirements. Put python first, then optionally sort alphabetically."""
     reqs_list = list(reqs)
 
     def is_python(req: str) -> bool:
@@ -552,7 +552,9 @@ def sort_reqs(reqs: Iterable[str]) -> List[str]:
 
     python_reqs = [req for req in reqs_list if is_python(req)]
     non_python_reqs = [req for req in reqs_list if not is_python(req)]
-    result = python_reqs + sorted(non_python_reqs)
+    if alphabetize:
+        non_python_reqs.sort()
+    result = python_reqs + non_python_reqs
     return result
 
 
diff --git a/grayskull/utils.py b/grayskull/utils.py
index 02155ca..d9d6c42 100644
--- a/grayskull/utils.py
+++ b/grayskull/utils.py
@@ -118,20 +118,27 @@ def string_similarity(a, b):
 def rm_duplicated_deps(all_requirements: Union[list, set, None]) -> Optional[list]:
     if not all_requirements:
         return None
-    new_value = []
-
     # Keep track of requirements which have already been added to the list.
-    # Here we store a canonicalized version of the requirement: lowercase,
-    # and underscores converted to dashes.
-    added_reqs = set()
+    # The key is a canonicalized version of the requirement: lowercase,
+    # and underscores converted to dashes. The value is the requirement itself,
+    # as it should be added.
+    # (This is order-preserving since dicts are ordered by first insertion.)
+    new_reqs: dict[str, str] = {}
 
     for dep in all_requirements:
         canonicalized = dep.replace("_", "-").lower()
-        if canonicalized in added_reqs:
-            continue
-        new_value.append(dep)
-        added_reqs.add(canonicalized)
-    return new_value
+        if canonicalized in new_reqs:
+            # In order to break ties deterministically, we prioritize the requirement
+            # which is alphanumerically lowest. This happens to prioritize the "-"
+            # character over "_".
+            # Example: given "importlib_metadata" and "importlib-metadata", we will
+            # keep "importlib-metadata" because it is alphabetically lower.
+            previous_req = new_reqs[canonicalized]
+            if dep < previous_req:
+                new_reqs[canonicalized] = dep
+        else:
+            new_reqs[canonicalized] = dep
+    return list(new_reqs.values())
 
 
 def format_dependencies(all_dependencies: List, name: str) -> List:


Test patch:
diff --git a/tests/test_pypi.py b/tests/test_pypi.py
index e76fb49..2caccd2 100644
--- a/tests/test_pypi.py
+++ b/tests/test_pypi.py
@@ -822,11 +822,13 @@ def test_zipp_recipe_tags_on_deps():
     config = Configuration(name="zipp", version="3.0.0")
     recipe = GrayskullFactory.create_recipe("pypi", config)
     assert recipe["build"]["noarch"]
-    assert recipe["requirements"]["host"] == [
-        "python >=3.6",
-        "pip",
-        "setuptools-scm >=3.4.1",
-    ]
+    assert sorted(recipe["requirements"]["host"]) == sorted(
+        [
+            "python >=3.6",
+            "pip",
+            "setuptools-scm >=3.4.1",
+        ]
+    )
 
 
 @pytest.mark.parametrize(
@@ -973,16 +975,18 @@ def test_panel_entry_points(tmpdir):
 def test_deps_comments():
     config = Configuration(name="kubernetes_asyncio", version="11.2.0")
     recipe = GrayskullFactory.create_recipe("pypi", config)
-    assert recipe["requirements"]["run"] == [
-        "python",
-        "aiohttp >=2.3.10,<4.0.0",
-        "certifi >=14.05.14",
-        "python-dateutil >=2.5.3",
-        "pyyaml >=3.12",
-        "setuptools >=21.0.0",
-        "six >=1.9.0",
-        "urllib3 >=1.24.2",
-    ]
+    assert sorted(recipe["requirements"]["run"]) == sorted(
+        [
+            "python",
+            "aiohttp >=2.3.10,<4.0.0",
+            "certifi >=14.05.14",
+            "python-dateutil >=2.5.3",
+            "pyyaml >=3.12",
+            "setuptools >=21.0.0",
+            "six >=1.9.0",
+            "urllib3 >=1.24.2",
+        ]
+    )
 
 
 @pytest.mark.github
@@ -1013,19 +1017,23 @@ def test_multiples_exit_setup():
 
 def test_sequence_inside_another_in_dependencies():
     recipe = create_python_recipe("unittest2=1.1.0", is_strict_cf=True)[0]
-    assert recipe["requirements"]["host"] == [
-        "python >=3.6",
-        "argparse",
-        "pip",
-        "six >=1.4",
-        "traceback2",
-    ]
-    assert recipe["requirements"]["run"] == [
-        "python >=3.6",
-        "argparse",
-        "six >=1.4",
-        "traceback2",
-    ]
+    assert sorted(recipe["requirements"]["host"]) == sorted(
+        [
+            "python >=3.6",
+            "argparse",
+            "pip",
+            "six >=1.4",
+            "traceback2",
+        ]
+    )
+    assert sorted(recipe["requirements"]["run"]) == sorted(
+        [
+            "python >=3.6",
+            "argparse",
+            "six >=1.4",
+            "traceback2",
+        ]
+    )
 
 
 def test_recipe_with_just_py_modules():
@@ -1300,13 +1308,14 @@ def test_cpp_language_extra():
 
 
 def test_sort_reqs():
-    assert sort_reqs(["pandas >=1.0", "numpy", "python"]) == [
-        "python",
-        "numpy",
-        "pandas >=1.0",
-    ]
-    assert sort_reqs(["pandas >=1.0", "numpy", "python >=3.8"]) == [
-        "python >=3.8",
-        "numpy",
-        "pandas >=1.0",
-    ]
+    # There are currently two acceptable sortings. Original ordering or alphabetical.
+    # In either sorting, 'python' always comes first.
+    original_deps = ["pandas >=1.0", "numpy", "python", "scipy"]
+    original_deps_38 = ["pandas >=1.0", "numpy", "python >=3.8", "scipy"]
+    sorted_deps_orig = ["python", "pandas >=1.0", "numpy", "scipy"]
+    sorted_deps_alpha = ["python", "numpy", "pandas >=1.0", "scipy"]
+    sorted_deps_orig_38 = ["python >=3.8", "pandas >=1.0", "numpy", "scipy"]
+    sorted_deps_alpha_38 = ["python >=3.8", "numpy", "pandas >=1.0", "scipy"]
+
+    assert sort_reqs(original_deps) in [sorted_deps_orig, sorted_deps_alpha]
+    assert sort_reqs(original_deps_38) in [sorted_deps_orig_38, sorted_deps_alpha_38]
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 7070b7e..a087579 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -123,4 +123,5 @@ def test_merge_dict_of_lists_item():
 
 def test_rm_duplicated_deps():
     assert rm_duplicated_deps([]) is None
-    assert rm_duplicated_deps(["my_craZy-pkg", "my-crazy-pkg"]) == ["my_craZy-pkg"]
+    # my-crazy-pkg is preferred because "my-crazy-pkg" < "my_craZy-pkg":
+    assert rm_duplicated_deps(["my_craZy-pkg", "my-crazy-pkg"]) == ["my-crazy-pkg"]