diff --git a/tests/resolver/test_filtering.py b/tests/resolver/test_filtering.py index 0b9ea39..50f59bf 100644 --- a/tests/resolver/test_filtering.py +++ b/tests/resolver/test_filtering.py @@ -7,6 +7,7 @@ from variantlib.models.variant import VariantDescription from variantlib.models.variant import VariantFeature from variantlib.models.variant import VariantProperty +from variantlib.resolver.filtering import filter_unsupported_feature_values from variantlib.resolver.filtering import filter_variants_by_features from variantlib.resolver.filtering import filter_variants_by_namespaces from variantlib.resolver.filtering import filter_variants_by_property @@ -456,3 +457,60 @@ def test_filter_variants_by_property_validation_error( ), maxlen=0, ) + + +# ====================== `filter_unsupported_feature_values` ====================== # + + +def test_filter_unsupported_feature_values() -> None: + vprop11 = VariantProperty("ns1", "f1", "v1") + vprop12 = VariantProperty("ns1", "f1", "v2") + vprop21 = VariantProperty("ns1", "f2", "v1") + vprop22 = VariantProperty("ns1", "f2", "v2") + vdescs = [ + VariantDescription(label="t1", properties=[vprop11, vprop12, vprop21]), + VariantDescription(label="t2", properties=[vprop11, vprop21, vprop22]), + VariantDescription(label="t3", properties=[vprop11, vprop21]), + ] + + assert list( + filter_unsupported_feature_values(vdescs, allowed_properties=[vprop11, vprop21]) + ) == [ + VariantDescription(label="t1", properties=[vprop11, vprop21]), + VariantDescription(label="t2", properties=[vprop11, vprop21]), + VariantDescription(label="t3", properties=[vprop11, vprop21]), + ] + + assert list( + filter_unsupported_feature_values( + vdescs, allowed_properties=[vprop11, vprop12, vprop21, vprop22] + ) + ) == [ + VariantDescription(label="t1", properties=[vprop11, vprop12, vprop21]), + VariantDescription(label="t2", properties=[vprop11, vprop21, vprop22]), + VariantDescription(label="t3", properties=[vprop11, vprop21]), + ] + + assert list( + filter_unsupported_feature_values( + vdescs, allowed_properties=[vprop11, vprop21, vprop22] + ) + ) == [ + VariantDescription(label="t1", properties=[vprop11, vprop21]), + VariantDescription(label="t2", properties=[vprop11, vprop21, vprop22]), + VariantDescription(label="t3", properties=[vprop11, vprop21]), + ] + + with pytest.raises( + ValidationError, match=r"None of `ns1 :: f2` values are allowed" + ): + list( + filter_unsupported_feature_values( + vdescs, allowed_properties=[vprop11, vprop22] + ) + ) + + with pytest.raises( + ValidationError, match=r"None of `ns1 :: f1` values are allowed" + ): + list(filter_unsupported_feature_values(vdescs, allowed_properties=[vprop22])) diff --git a/tests/resolver/test_lib.py b/tests/resolver/test_lib.py index a1e0290..b50ddbb 100644 --- a/tests/resolver/test_lib.py +++ b/tests/resolver/test_lib.py @@ -95,11 +95,16 @@ def vdescs(vprops: list[VariantProperty]) -> list[VariantDescription]: vprop1, vprop2, vprop3, vprop4, vprop5, vprop6 = vprops # fmt: off - # Important: vprop4 and vprop5 are mutually exclusive return [ + VariantDescription( + [vprop1, vprop2, vprop3, vprop4, vprop5, vprop6], label="ba" + ), + # variants with 5 properties VariantDescription([vprop1, vprop2, vprop3, vprop4, vprop6], label="a"), + VariantDescription([vprop1, vprop2, vprop4, vprop5, vprop6], label="bc"), VariantDescription([vprop1, vprop2, vprop3, vprop5, vprop6], label="b"), + VariantDescription([vprop1, vprop3, vprop4, vprop5, vprop6], label="bc"), # variants with 4 properties VariantDescription([vprop1, vprop2, vprop3, vprop4], label="c"), # - vprop6 @@ -107,9 +112,11 @@ def vdescs(vprops: list[VariantProperty]) -> list[VariantDescription]: VariantDescription([vprop1, vprop2, vprop3, vprop6], label="c"), # - vprop4/5 + VariantDescription([vprop1, vprop2, vprop4, vprop5], label="bd"), VariantDescription([vprop1, vprop2, vprop4, vprop6], label="d"), # - vprop3 VariantDescription([vprop1, vprop2, vprop5, vprop6], label="e"), # - vprop3 + VariantDescription([vprop1, vprop3, vprop4, vprop5], label="be"), VariantDescription([vprop1, vprop3, vprop4, vprop6], label="f"), # - vprop2 VariantDescription([vprop1, vprop3, vprop5, vprop6], label="g"), # - vprop2 @@ -127,6 +134,7 @@ def vdescs(vprops: list[VariantProperty]) -> list[VariantDescription]: VariantDescription([vprop1, vprop3, vprop5], label="o"), VariantDescription([vprop1, vprop3, vprop6], label="p"), + VariantDescription([vprop1, vprop4, vprop5], label="bf"), VariantDescription([vprop1, vprop4, vprop6], label="q"), VariantDescription([vprop1, vprop5, vprop6], label="r"), @@ -453,8 +461,9 @@ def test_filter_variants_remove_properties( # =================== `sort_and_filter_supported_variants` ================== # +@pytest.mark.parametrize("filter_values", [False, True]) def test_sort_and_filter_supported_variants( - vdescs: list[VariantDescription], vprops: list[VariantProperty] + vdescs: list[VariantDescription], vprops: list[VariantProperty], filter_values: bool ) -> None: assert len(vprops) == 6 @@ -497,6 +506,8 @@ def test_sort_and_filter_supported_variants( # 1. Everything with vprop6 # 1.1. + vprop3 # 1.1.1. + vprop5 + # 1.1.1.1. + vprop4 + VariantDescription([vprop1, vprop3, vprop4, vprop5, vprop6], label="bc"), VariantDescription([vprop1, vprop3, vprop5, vprop6], label="g"), VariantDescription([vprop3, vprop5, vprop6], label="y"), # 1.1.2. + vprop4 @@ -519,6 +530,8 @@ def test_sort_and_filter_supported_variants( # 2. Everything with vprop3 # 2.1. + vprop5 + # 2.1.1. + vprop4 + VariantDescription([vprop1, vprop3, vprop4, vprop5], label="be"), VariantDescription([vprop1, vprop3, vprop5], label="o"), VariantDescription([vprop3, vprop5], label="aj"), # 2.2. + vprop4 @@ -531,6 +544,7 @@ def test_sort_and_filter_supported_variants( # 3. vprop5 VariantDescription([vprop1, vprop5], label="ac"), + VariantDescription([vprop1, vprop4, vprop5], label="bf"), VariantDescription([vprop5], label="ar"), # 4. vprop4 @@ -555,6 +569,112 @@ def test_sort_and_filter_supported_variants( property_priorities=prio_vprops, feature_priorities=prio_vfeats, namespace_priorities=prio_namespaces, + filter_values=filter_values, + ) + == expected_vdescs + ) + + +@pytest.mark.parametrize("filter_values", [False, True]) +def test_sort_and_filter_supported_variants_no_vprop5( + vdescs: list[VariantDescription], vprops: list[VariantProperty], filter_values: bool +) -> None: + assert len(vprops) == 6 + + vprop1, vprop2, vprop3, vprop4, vprop5, vprop6 = vprops + + # ~~~~~~~~~~~~~~~~~~~~~~~~~~~ SORTING PARAMETERS ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ # + + prio_vprops = {"tyrell_corp": {"feat_b": ["efghij"]}} + + prio_vfeats = {"tyrell_corp": ["feat_c"]} + + prio_namespaces = ["NotExistingNamespace", "tyrell_corp", "omnicorp"] + + # Sanity check variant ordering: + # 1. vprop6. tyrell_corp :: feat_c + # 2. vprop3. tyrell_corp :: feat_a + # 3. vprop5. tyrell_corp :: feat_b :: efghij + # 4. vprop4. tyrell_corp :: feat_b :: abcde + # 5. vprop1. omnicorp :: feat_a + # 6. vprop2. omnicorp :: feat_b + + assert sort_variant_properties( + vprops=vprops, + namespace_priorities=prio_namespaces, + feature_priorities=prio_vfeats, + property_priorities=prio_vprops, + ) == [vprop6, vprop3, vprop5, vprop4, vprop1, vprop2] + + # Default Ordering: properties are assumed pre-sorted in features/properties + # vprop1 > vprop2 > vprop3 > vprop4 > vprop5 > vprop6 + # Note: Namespace is already accounted for in 3) + + # Last Preferential Order: More features are preferred over less features + + # ----------------------------------------------------------------------------- # + + m_vprop5 = [] if filter_values else [vprop5] + + # fmt: off + expected_vdescs = [ + # Effective vdesc order: + # 1. Everything with vprop6 + # 1.1. + vprop3 + # 1.1.1. + vprop5 + # 1.1.1.1. + vprop4 + VariantDescription([vprop1, vprop3, vprop4, *m_vprop5, vprop6], label="bc"), + # 1.1.2. + vprop4 + VariantDescription([vprop1, vprop3, vprop4, vprop6], label="f"), + VariantDescription([vprop3, vprop4, vprop6], label="x"), + # 1.1.3. + vprop1 + VariantDescription([vprop1, vprop3, vprop6], label="p"), + # 1.1.4. vprop6 + vprop3 + VariantDescription([vprop3, vprop6], label="ak"), + # 1.3. + vprop4 + VariantDescription([vprop1, vprop4, vprop6], label="q"), + VariantDescription([vprop4, vprop6], label="al"), + # 1.4. + vprop1 + VariantDescription([vprop1, vprop6], label="ad"), + # 1. sole vprop6 + VariantDescription([vprop6], label="as"), + + # 2. Everything with vprop3 + # 2.1. + vprop5 + # 2.1.1. + vprop4 + VariantDescription([vprop1, vprop3, vprop4, *m_vprop5], label="be"), + # 2.2. + vprop4 + VariantDescription([vprop1, vprop3, vprop4], label="n"), + VariantDescription([vprop3, vprop4], label="ai"), + # 2.3. + vprop1 + VariantDescription([vprop1, vprop3], label="aa"), + # 2. sole vprop3 + VariantDescription([vprop3], label="ap"), + + # 3. vprop1 + vprop4 + VariantDescription([vprop1, vprop4], label="ab"), + VariantDescription([vprop1, vprop4, *m_vprop5], label="bf"), + VariantDescription([vprop4], label="aq"), + + # 4. sole vprop1 + VariantDescription([vprop1], label="an"), + + # Null-Variant is never removed and last - Implicitly added + VariantDescription(), + ] + # fmt: on + + # Shuffling the list & creating duplicates + inputs_vdescs = shuffle_vdescs(vdescs=vdescs) + + assert ( + sort_and_filter_supported_variants( + vdescs=inputs_vdescs, + supported_vprops=[vprop1, vprop3, vprop4, vprop6], + property_priorities=prio_vprops, + feature_priorities=prio_vfeats, + namespace_priorities=prio_namespaces, + filter_values=filter_values, ) == expected_vdescs ) diff --git a/tests/test_api.py b/tests/test_api.py index 4d744e9..6dd1dda 100644 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -81,12 +81,10 @@ def configs( @pytest.mark.parametrize("construct", [False, True]) -@pytest.mark.parametrize("custom_labels", [False, True]) @pytest.mark.parametrize("explicit_null", [False, True]) def test_get_variants_by_priority_roundtrip( configs: list[ProviderConfig], construct: bool, - custom_labels: bool, explicit_null: bool, ) -> None: """Test that we can round-trip all combinations via variants.json and get the same @@ -116,9 +114,7 @@ def test_get_variants_by_priority_roundtrip( for namespace, plugin_api in plugin_apis.items() }, VARIANTS_JSON_VARIANT_DATA_KEY: { - f"foo{vdesc.hexdigest[:4]}" - if custom_labels and not vdesc.is_null_variant() - else get_variant_label(vdesc): vdesc.to_dict() + vdesc.label: vdesc.to_dict() for vdesc in combinations if explicit_null or not vdesc.is_null_variant() }, @@ -131,12 +127,7 @@ def test_get_variants_by_priority_roundtrip( # variants_json = VariantsJson(typed_variants_json) - assert get_variants_by_priority(variants_json=typed_variants_json) == [ - f"foo{vdesc.hexdigest[:4]}" - if custom_labels and not vdesc.is_null_variant() - else get_variant_label(vdesc) - for vdesc in combinations - ] + assert get_variants_by_priority(variants_json=typed_variants_json) == combinations @settings(deadline=None, suppress_health_check=[HealthCheck.function_scoped_fixture]) @@ -221,9 +212,7 @@ def get_or_skip_combinations() -> Generator[VariantDescription]: "variantlib.plugins.loader.BasePluginLoader.get_supported_configs" ).return_value = {provider_cfg.namespace: provider_cfg for provider_cfg in configs} - assert get_variants_by_priority(variants_json=typed_variants_json) == [ - get_variant_label(vdesc) for vdesc in combinations - ] + assert get_variants_by_priority(variants_json=typed_variants_json) == combinations @pytest.mark.parametrize( @@ -469,7 +458,10 @@ def test_check_variant_supported_dist( ) -> None: variant_json = VariantsJson(common_variant_info) variant_json.variants[vdesc.hexdigest] = vdesc - assert check_variant_supported(variant_info=variant_json) is expected + if expected: + assert check_variant_supported(variant_info=variant_json) == vdesc + else: + assert check_variant_supported(variant_info=variant_json) is None def test_check_variant_supported_generic() -> None: @@ -543,7 +535,7 @@ def test_get_variant_environment_dict() -> None: }, "variant_label": "foo", } - assert get_variant_environment_dict(vdesc, "foo") == expected + assert get_variant_environment_dict(vdesc) == expected def test_make_variant_dist_info_invalid_label(): diff --git a/variantlib/api.py b/variantlib/api.py index 6e73855..f6638fd 100644 --- a/variantlib/api.py +++ b/variantlib/api.py @@ -53,7 +53,7 @@ def get_variants_by_priority( variants_json: VariantsJsonDict | VariantsJson, venv_python_executable: str | pathlib.Path | None = None, enable_optional_plugins: bool | list[VariantNamespace] = False, -) -> list[str]: +) -> list[VariantDescription]: supported_vprops = [] if not isinstance(variants_json, VariantsJson): variants_json = VariantsJson(variants_json) @@ -80,27 +80,25 @@ def get_variants_by_priority( config = VariantConfiguration.get_config() - return [ - vdesc.label - for vdesc in sort_and_filter_supported_variants( - list(variants_json.variants.values()), - supported_vprops, - namespace_priorities=aggregate_namespace_priorities( - config.namespace_priorities, - variants_json.namespace_priorities, - ), - feature_priorities=aggregate_feature_priorities( - config.feature_priorities, - { - namespace: provider.feature_order - for namespace, provider in variants_json.providers.items() - }, - ), - property_priorities=aggregate_property_priorities( - config.property_priorities, - ), - ) - ] + return sort_and_filter_supported_variants( + list(variants_json.variants.values()), + supported_vprops, + namespace_priorities=aggregate_namespace_priorities( + config.namespace_priorities, + variants_json.namespace_priorities, + ), + feature_priorities=aggregate_feature_priorities( + config.feature_priorities, + { + namespace: provider.feature_order + for namespace, provider in variants_json.providers.items() + }, + ), + property_priorities=aggregate_property_priorities( + config.property_priorities, + ), + filter_values=True, + ) def validate_variant( @@ -281,10 +279,11 @@ def check_variant_supported( variant_info: VariantInfo, venv_python_executable: str | pathlib.Path | None = None, enable_optional_plugins: bool | list[VariantNamespace] = False, -) -> bool: - """Check if variant description is supported +) -> VariantDescription | None: + """Check if variant is supported and return filtered description - Returns True if the variant description is supported. + Returns a VariantDescription filtered down to supported values if it + is supported. Otherwise, returns None. If `vdesc` is provided, it is tested. Otherwise, `variant_info` must be a `DistMetadata` and variant description is inferred from it. @@ -319,19 +318,18 @@ def check_variant_supported( VariantConfiguration.get_config() - return bool( - list( - filter_variants( - vdescs=[vdesc], - allowed_properties=supported_vprops, - ) + filtered = list( + filter_variants( + vdescs=[vdesc], + allowed_properties=supported_vprops, + filter_values=True, ) ) + return filtered[0] if filtered else None def get_variant_environment_dict( variant_desc: VariantDescription, - variant_label: str | None = None, ) -> dict[str, set[str] | str]: """Get the dict for packaging Marker.evaluate()""" @@ -343,13 +341,6 @@ def get_variant_environment_dict( "variant_properties": {vprop.to_str() for vprop in variant_desc.properties}, "variant_label": variant_desc.label, } - if variant_label is not None: - warnings.warn( - "Passing variant_label is deprecated, provide VariantDescription() " - "with label instead", - stacklevel=2, - ) - ret["variant_label"] = variant_label return ret diff --git a/variantlib/resolver/filtering.py b/variantlib/resolver/filtering.py index f8c0aa0..46fdd3e 100644 --- a/variantlib/resolver/filtering.py +++ b/variantlib/resolver/filtering.py @@ -5,6 +5,7 @@ from collections.abc import Iterable from typing import TYPE_CHECKING +from variantlib.errors import ValidationError from variantlib.models.variant import VariantDescription from variantlib.models.variant import VariantFeature from variantlib.models.variant import VariantProperty @@ -221,3 +222,82 @@ def _should_include(vdesc: VariantDescription) -> bool: return True yield from filter(_should_include, vdescs) + + +def filter_unsupported_feature_values( + vdescs: Iterable[VariantDescription], + allowed_properties: list[VariantProperty], + forbidden_properties: list[VariantProperty] | None = None, +) -> Generator[VariantDescription]: + """ + Filters out unsupported from `VariantDescription`s. + + ** Implementation Note:** + - All of the provided `VariantDescription`s must be compatible (filter via + `filter_variants_by_property()` first. + - Installer will provide the list of allowed properties from variant provider + plugins. + - User can [optionally] provide a list of forbidden properties to be excluded. + Forbidden properties take precedence of "allowed properties" and will be excluded. + + :param vdescs: list of `VariantDescription` to filter. + :param allowed_properties: List of allowed `VariantProperty`. + :param forbidden_properties: List of forbidden `VariantProperty`. + :return: Filtered list of `VariantDescription`. + """ + + if forbidden_properties is None: + forbidden_properties = [] + + # Input validation + validate_type(vdescs, Iterable) + validate_type(allowed_properties, list[VariantProperty]) + validate_type(forbidden_properties, list[VariantProperty]) + + # for performance reasons we convert the list to a set to avoid O(n) lookups + forbidden_properties_hexs = {vprop.property_hash for vprop in forbidden_properties} + + # We filter out any properties that are in the forbidden list. + allowed_properties = list( + filter( + lambda vprop: vprop.property_hash not in forbidden_properties_hexs, + allowed_properties, + ) + ) + + # We group allowed properties by their namespace and feature: + # => only one match per group is required. + # Note: This step is required for the OR match within one VariantFeature + allowed_props_dict: dict[ + tuple[VariantNamespace, VariantFeatureValue], set[VariantFeatureValue] + ] = defaultdict(set) + for vprop in allowed_properties: + allowed_props_dict[(vprop.namespace, vprop.feature)].add(vprop.value) + + def _filter_vdesc(vdesc: VariantDescription) -> VariantDescription: + validate_type(vdesc, VariantDescription) + + vdesc_prop_dict: dict[ + tuple[VariantNamespace, VariantFeatureValue], set[VariantFeatureValue] + ] = defaultdict(set) + for vprop in vdesc.properties: + vdesc_prop_dict[(vprop.namespace, vprop.feature)].add(vprop.value) + + for ns, vfeat_name in list(vdesc_prop_dict): + allowed_props = allowed_props_dict.get((ns, vfeat_name), set()) + vdesc_prop_dict[(ns, vfeat_name)] &= allowed_props + if not vdesc_prop_dict[(ns, vfeat_name)]: + raise ValidationError( + f"None of `{ns} :: {vfeat_name}` values are allowed in {vdesc!r}" + ) + + return VariantDescription( + label=vdesc.label, + properties=[ + VariantProperty(ns, vfeat_name, vfeat_value) + for (ns, vfeat_name), values in vdesc_prop_dict.items() + for vfeat_value in values + ], + ) + + yield from map(_filter_vdesc, vdescs) diff --git a/variantlib/resolver/lib.py b/variantlib/resolver/lib.py index e06a04f..ea45eb5 100644 --- a/variantlib/resolver/lib.py +++ b/variantlib/resolver/lib.py @@ -12,6 +12,7 @@ from variantlib.models.variant import VariantDescription from variantlib.models.variant import VariantFeature from variantlib.models.variant import VariantProperty +from variantlib.resolver.filtering import filter_unsupported_feature_values from variantlib.resolver.filtering import filter_variants_by_features from variantlib.resolver.filtering import filter_variants_by_namespaces from variantlib.resolver.filtering import filter_variants_by_property @@ -47,6 +48,7 @@ def filter_variants( forbidden_namespaces: list[str] | None = None, forbidden_features: list[VariantFeature] | None = None, forbidden_properties: list[VariantProperty] | None = None, + filter_values: bool = False, ) -> Generator[VariantDescription]: """ Filters out a `list` of `VariantDescription` with the following filters: @@ -57,12 +59,14 @@ def filter_variants( - Forbidden `variant namespaces` removed - if `forbidden_namespaces` is not None - Forbidden `variant features` removed - if `forbidden_features` is not None - Forbidden `variant properties` removed - if `forbidden_properties` is not None + - Unsupported values filtered out if `filter_values` is True. :param vdescs: list of `VariantDescription` to filter. :param allowed_properties: List of allowed `VariantProperty`. :param forbidden_namespaces: List of forbidden variant namespaces as `str`. :param forbidden_features: List of forbidden `VariantFeature`. :param forbidden_properties: List of forbidden `VariantProperty`. + :param filter_values: Should values be filtered? :return: Filtered list of `VariantDescription`. """ @@ -110,6 +114,15 @@ def filter_variants( forbidden_properties=forbidden_properties, ) + # Step 4 [Optional] + # Remove unsupported feature values + if filter_values: + result = filter_unsupported_feature_values( + vdescs=result, + allowed_properties=allowed_properties, + forbidden_properties=forbidden_properties, + ) + yield from result @@ -180,6 +193,7 @@ def sort_and_filter_supported_variants( forbidden_namespaces: list[VariantNamespace] | None = None, forbidden_features: list[VariantFeature] | None = None, forbidden_properties: list[VariantProperty] | None = None, + filter_values: bool = False, ) -> list[VariantDescription]: """ Sort and filter a list of `VariantDescription` objects based on their @@ -190,6 +204,7 @@ def sort_and_filter_supported_variants( :param namespace_priorities: Ordered list of `str` objects. :param feature_priorities: Ordered list of `VariantFeature` objects. :param property_priorities: Ordered list of `VariantProperty` objects. + :param filter_values: Should values be filtered? :return: Sorted and filtered list of `VariantDescription` objects. """ @@ -240,6 +255,7 @@ def sort_and_filter_supported_variants( forbidden_namespaces=forbidden_namespaces, forbidden_features=forbidden_features, forbidden_properties=forbidden_properties, + filter_values=filter_values, ) )