From 6f0474b337583ef357e0244339df116d921940a4 Mon Sep 17 00:00:00 2001 From: Marcel Bargull Date: Tue, 27 Feb 2024 08:18:05 +0100 Subject: [PATCH 1/5] Remove/avoid redundant function calls Signed-off-by: Marcel Bargull --- conda_build/metadata.py | 12 +++++------- conda_build/variants.py | 16 +++++++++------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/conda_build/metadata.py b/conda_build/metadata.py index 01f3367d03..af0ca0588f 100644 --- a/conda_build/metadata.py +++ b/conda_build/metadata.py @@ -2641,13 +2641,11 @@ def get_loop_vars(self): return variants.get_vars(_variants, loop_only=True) def get_used_loop_vars(self, force_top_level=False, force_global=False): - return { - var - for var in self.get_used_vars( - force_top_level=force_top_level, force_global=force_global - ) - if var in self.get_loop_vars() - } + loop_vars = self.get_loop_vars() + used_vars = self.get_used_vars( + force_top_level=force_top_level, force_global=force_global + ) + return set(loop_vars).intersection(used_vars) def get_rendered_recipe_text( self, permit_undefined_jinja=False, extract_pattern=None diff --git a/conda_build/variants.py b/conda_build/variants.py index c5bbe9a41e..f39c614715 100644 --- a/conda_build/variants.py +++ b/conda_build/variants.py @@ -697,16 +697,18 @@ def get_package_variants(recipedir_or_metadata, config=None, variants=None): def get_vars(variants, loop_only=False): """For purposes of naming/identifying, provide a way of identifying which variables contribute to the matrix dimensionality""" - special_keys = {"pin_run_as_build", "zip_keys", "ignore_version"} - special_keys.update(set(ensure_list(variants[0].get("extend_keys")))) + first_variant, *other_variants = variants + special_keys = { + "pin_run_as_build", + "zip_keys", + "ignore_version", + *ensure_list(first_variant.get("extend_keys")), + } loop_vars = [ k - for k in variants[0] + for k, v in first_variant.items() if k not in special_keys - and ( - not loop_only - or any(variant[k] != variants[0][k] for variant in variants[1:]) - ) + and (not loop_only or any(variant[k] != v for variant in other_variants)) ] return loop_vars From a65dfdd4551928f44889c5d91f29beabf4440aa2 Mon Sep 17 00:00:00 2001 From: Ken Odegard Date: Tue, 16 Apr 2024 23:12:27 -0500 Subject: [PATCH 2/5] Refactor get_vars --- conda_build/variants.py | 24 +++++++++++++++++------- tests/test_variants.py | 16 ++++++++++++++++ 2 files changed, 33 insertions(+), 7 deletions(-) diff --git a/conda_build/variants.py b/conda_build/variants.py index f39c614715..2dca13b48f 100644 --- a/conda_build/variants.py +++ b/conda_build/variants.py @@ -3,6 +3,8 @@ """This file handles the parsing of feature specifications from files, ending up with a configuration matrix""" +from __future__ import annotations + import os.path import re import sys @@ -10,6 +12,7 @@ from copy import copy from functools import lru_cache from itertools import product +from typing import TYPE_CHECKING import yaml from conda.base.context import context @@ -18,6 +21,9 @@ from .utils import ensure_list, get_logger, islist, on_win, trim_empty_keys from .version import _parse as parse_version +if TYPE_CHECKING: + from typing import Any, Iterable + DEFAULT_VARIANTS = { "python": f"{sys.version_info.major}.{sys.version_info.minor}", "numpy": { @@ -694,7 +700,7 @@ def get_package_variants(recipedir_or_metadata, config=None, variants=None): return filter_combined_spec_to_used_keys(combined_spec, specs=specs) -def get_vars(variants, loop_only=False): +def get_vars(variants: Iterable[dict[str, Any]], loop_only: bool = False) -> set[str]: """For purposes of naming/identifying, provide a way of identifying which variables contribute to the matrix dimensionality""" first_variant, *other_variants = variants @@ -704,12 +710,16 @@ def get_vars(variants, loop_only=False): "ignore_version", *ensure_list(first_variant.get("extend_keys")), } - loop_vars = [ - k - for k, v in first_variant.items() - if k not in special_keys - and (not loop_only or any(variant[k] != v for variant in other_variants)) - ] + loop_vars = set(first_variant) - special_keys + if loop_only: + loop_vars = { + var + for var in loop_vars + if any( + first_variant[var] != other_variant[var] + for other_variant in other_variants + ) + } return loop_vars diff --git a/tests/test_variants.py b/tests/test_variants.py index 50e9cea4f2..1ea0b6f142 100644 --- a/tests/test_variants.py +++ b/tests/test_variants.py @@ -18,6 +18,7 @@ dict_of_lists_to_list_of_dicts, filter_combined_spec_to_used_keys, get_package_variants, + get_vars, validate_spec, ) @@ -700,3 +701,18 @@ def test_zip_key_filtering( } assert filter_combined_spec_to_used_keys(combined_spec, specs=specs) == expected + + +def test_get_vars(): + variants = [ + { + "python": "3.12", + "nodejs": "20", + "zip_keys": [], # ignored + }, + {"python": "3.12", "nodejs": "18"}, + {"python": "3.12", "nodejs": "20"}, + ] + + assert get_vars(variants, loop_only=False) == {"python", "nodejs"} + assert get_vars(variants, loop_only=True) == {"nodejs"} From 40de0ae1c195ef9c3e6974630d576c96ea64506a Mon Sep 17 00:00:00 2001 From: Ken Odegard Date: Tue, 16 Apr 2024 23:15:44 -0500 Subject: [PATCH 3/5] Deprecate get_vars(loop_only) --- conda_build/metadata.py | 2 +- conda_build/variants.py | 24 ++++++++++-------------- tests/test_variants.py | 3 +-- 3 files changed, 12 insertions(+), 17 deletions(-) diff --git a/conda_build/metadata.py b/conda_build/metadata.py index af0ca0588f..aab3c34480 100644 --- a/conda_build/metadata.py +++ b/conda_build/metadata.py @@ -2638,7 +2638,7 @@ def get_loop_vars(self): if hasattr(self.config, "input_variants") else self.config.variants ) - return variants.get_vars(_variants, loop_only=True) + return variants.get_vars(_variants) def get_used_loop_vars(self, force_top_level=False, force_global=False): loop_vars = self.get_loop_vars() diff --git a/conda_build/variants.py b/conda_build/variants.py index 2dca13b48f..2ea4091b88 100644 --- a/conda_build/variants.py +++ b/conda_build/variants.py @@ -18,6 +18,7 @@ from conda.base.context import context from .conda_interface import cc_conda_build +from .deprecations import deprecated from .utils import ensure_list, get_logger, islist, on_win, trim_empty_keys from .version import _parse as parse_version @@ -700,27 +701,22 @@ def get_package_variants(recipedir_or_metadata, config=None, variants=None): return filter_combined_spec_to_used_keys(combined_spec, specs=specs) -def get_vars(variants: Iterable[dict[str, Any]], loop_only: bool = False) -> set[str]: +@deprecated.argument("24.5", "24.7", "loop_only") +def get_vars(variants: Iterable[dict[str, Any]]) -> set[str]: """For purposes of naming/identifying, provide a way of identifying which variables contribute to the matrix dimensionality""" - first_variant, *other_variants = variants + first, *others = variants special_keys = { "pin_run_as_build", "zip_keys", "ignore_version", - *ensure_list(first_variant.get("extend_keys")), + *ensure_list(first.get("extend_keys")), + } + return { + var + for var in set(first) - special_keys + if any(first[var] != other[var] for other in others) } - loop_vars = set(first_variant) - special_keys - if loop_only: - loop_vars = { - var - for var in loop_vars - if any( - first_variant[var] != other_variant[var] - for other_variant in other_variants - ) - } - return loop_vars @lru_cache(maxsize=None) diff --git a/tests/test_variants.py b/tests/test_variants.py index 1ea0b6f142..71b2e7e627 100644 --- a/tests/test_variants.py +++ b/tests/test_variants.py @@ -714,5 +714,4 @@ def test_get_vars(): {"python": "3.12", "nodejs": "20"}, ] - assert get_vars(variants, loop_only=False) == {"python", "nodejs"} - assert get_vars(variants, loop_only=True) == {"nodejs"} + assert get_vars(variants) == {"nodejs"} From 6a609ee8ae87a17feed31dc221fe24401dd90e31 Mon Sep 17 00:00:00 2001 From: Ken Odegard Date: Tue, 16 Apr 2024 23:16:47 -0500 Subject: [PATCH 4/5] Add news --- news/5280-deprecate-get_vars-loop_only | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 news/5280-deprecate-get_vars-loop_only diff --git a/news/5280-deprecate-get_vars-loop_only b/news/5280-deprecate-get_vars-loop_only new file mode 100644 index 0000000000..e18d5cfe8c --- /dev/null +++ b/news/5280-deprecate-get_vars-loop_only @@ -0,0 +1,19 @@ +### Enhancements + +* + +### Bug fixes + +* + +### Deprecations + +* Deprecate `conda_build.variants.get_vars(loop_only)`. Unused. (#5280) + +### Docs + +* + +### Other + +* From 210c30ac39477b927f7b70e07a9b52fc6f12b134 Mon Sep 17 00:00:00 2001 From: Ken Odegard Date: Tue, 16 Apr 2024 23:19:10 -0500 Subject: [PATCH 5/5] Adjust imports --- conda_build/metadata.py | 52 +++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 30 deletions(-) diff --git a/conda_build/metadata.py b/conda_build/metadata.py index aab3c34480..b73f3519bb 100644 --- a/conda_build/metadata.py +++ b/conda_build/metadata.py @@ -19,7 +19,7 @@ from conda.base.context import context from conda.gateways.disk.read import compute_sum -from . import exceptions, utils, variants +from . import exceptions, utils from .conda_interface import MatchSpec from .config import Config, get_or_merge_config from .features import feature_list @@ -34,6 +34,15 @@ insert_variant_versions, on_win, ) +from .variants import ( + dict_of_lists_to_list_of_dicts, + find_used_variables_in_batch_script, + find_used_variables_in_shell_script, + find_used_variables_in_text, + get_default_variant, + get_vars, + list_of_dicts_to_dict_of_lists, +) if TYPE_CHECKING: from typing import Literal @@ -156,7 +165,7 @@ def get_selectors(config: Config) -> dict[str, bool]: if arch == "32": d["x86"] = plat.endswith(("-32", "-64")) - defaults = variants.get_default_variant(config) + defaults = get_default_variant(config) py = config.variant.get("python", defaults["python"]) # there are times when python comes in as a tuple if not hasattr(py, "split"): @@ -2448,9 +2457,7 @@ def append_parent_metadata(self, out_metadata): def get_reduced_variant_set(self, used_variables): # reduce variable space to limit work we need to do - full_collapsed_variants = variants.list_of_dicts_to_dict_of_lists( - self.config.variants - ) + full_collapsed_variants = list_of_dicts_to_dict_of_lists(self.config.variants) reduced_collapsed_variants = full_collapsed_variants.copy() reduce_keys = set(self.config.variants[0].keys()) - set(used_variables) @@ -2482,7 +2489,7 @@ def get_reduced_variant_set(self, used_variables): # save only one element from this key reduced_collapsed_variants[key] = utils.ensure_list(next(iter(values))) - out = variants.dict_of_lists_to_list_of_dicts(reduced_collapsed_variants) + out = dict_of_lists_to_list_of_dicts(reduced_collapsed_variants) return out def get_output_metadata_set( @@ -2633,12 +2640,7 @@ def get_output_metadata_set( return output_tuples def get_loop_vars(self): - _variants = ( - self.config.input_variants - if hasattr(self.config, "input_variants") - else self.config.variants - ) - return variants.get_vars(_variants) + return get_vars(getattr(self.config, "input_variants", self.config.variants)) def get_used_loop_vars(self, force_top_level=False, force_global=False): loop_vars = self.get_loop_vars() @@ -2825,7 +2827,7 @@ def _get_used_vars_meta_yaml(self, force_top_level=False, force_global=False): apply_selectors=False, ) - all_used_selectors = variants.find_used_variables_in_text( + all_used_selectors = find_used_variables_in_text( variant_keys, recipe_text, selectors_only=True ) @@ -2834,7 +2836,7 @@ def _get_used_vars_meta_yaml(self, force_top_level=False, force_global=False): force_global=force_global, apply_selectors=True, ) - all_used_reqs = variants.find_used_variables_in_text( + all_used_reqs = find_used_variables_in_text( variant_keys, recipe_text, selectors_only=False ) @@ -2845,9 +2847,7 @@ def _get_used_vars_meta_yaml(self, force_top_level=False, force_global=False): if force_global: used = all_used else: - requirements_used = variants.find_used_variables_in_text( - variant_keys, reqs_text - ) + requirements_used = find_used_variables_in_text(variant_keys, reqs_text) outside_reqs_used = all_used - requirements_used requirements_used = trim_build_only_deps(self, requirements_used) @@ -2860,16 +2860,12 @@ def _get_used_vars_build_scripts(self): buildsh = os.path.join(self.path, "build.sh") if os.path.isfile(buildsh): used_vars.update( - variants.find_used_variables_in_shell_script( - self.config.variant, buildsh - ) + find_used_variables_in_shell_script(self.config.variant, buildsh) ) bldbat = os.path.join(self.path, "bld.bat") if self.config.platform == "win" and os.path.isfile(bldbat): used_vars.update( - variants.find_used_variables_in_batch_script( - self.config.variant, bldbat - ) + find_used_variables_in_batch_script(self.config.variant, bldbat) ) return used_vars @@ -2882,15 +2878,11 @@ def _get_used_vars_output_script(self): script = os.path.join(self.path, this_output["script"]) if os.path.splitext(script)[1] == ".sh": used_vars.update( - variants.find_used_variables_in_shell_script( - self.config.variant, script - ) + find_used_variables_in_shell_script(self.config.variant, script) ) elif os.path.splitext(script)[1] == ".bat": used_vars.update( - variants.find_used_variables_in_batch_script( - self.config.variant, script - ) + find_used_variables_in_batch_script(self.config.variant, script) ) else: log = utils.get_logger(__name__) @@ -2901,7 +2893,7 @@ def _get_used_vars_output_script(self): return used_vars def get_variants_as_dict_of_lists(self): - return variants.list_of_dicts_to_dict_of_lists(self.config.variants) + return list_of_dicts_to_dict_of_lists(self.config.variants) def clean(self): """This ensures that clean is called with the correct build id"""