Skip to content
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.

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

feat: Correct behavior of matrix fallback priority, error on duplicate matrix selectors. #110

Merged
merged 4 commits into from
Sep 23, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,20 @@ def make_dependency_files(
if file_type not in specific_entry.output_types:
continue

found = False
# Ensure that all specific matrices are unique
num_matrices = len(specific_entry.matrices)
num_unique = len(
{
frozenset(specific_matrices_entry.matrix.items())
for specific_matrices_entry in specific_entry.matrices
}
)
if num_matrices != num_unique:
err = f"All matrix entries must be unique. Found duplicates in '{include}':"
for specific_matrices_entry in specific_entry.matrices:
err += f"\n - {specific_matrices_entry.matrix}"
raise ValueError(err)

fallback_entry = None
for specific_matrices_entry in specific_entry.matrices:
# An empty `specific_matrices_entry["matrix"]` is
Expand All @@ -403,18 +416,12 @@ def make_dependency_files(
continue

if should_use_specific_entry(matrix_combo, specific_matrices_entry.matrix):
# Raise an error if multiple specific entries
# (not including the fallback_entry) match a
# requested matrix combination.
if found:
raise ValueError(f"Found multiple matches for matrix {matrix_combo}")
found = True
# A package list may be empty as a way to
# indicate that for some matrix elements no
# packages should be installed.
dependencies.extend(specific_matrices_entry.packages or [])

if not found:
break
else:
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On the off chance that this is new to you: Python has a for/else construct that can be used to simplify things where you would otherwise need a loop with a "found" variable. It doesn't come up often in Python usage, but it's nice when it fits the problem cleanly. I was able to eliminate the found variable here with that approach.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing this out, this was new to me.

if fallback_entry:
dependencies.extend(fallback_entry.packages)
else:
Expand Down
24 changes: 24 additions & 0 deletions tests/examples/duplicate-specific-matrix-entries/dependencies.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
files:
all:
output: conda
conda_dir: output/actual
matrix:
cuda: ["11.5", "11.8"]
includes:
- cudatoolkit
channels:
- rapidsai
- conda-forge
dependencies:
cudatoolkit:
specific:
- output_types: conda
matrices:
- matrix:
cuda: "11.5"
packages:
- cudatoolkit=11.5
- matrix:
cuda: "11.5"
packages:
- cudatoolkit=11.5
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
files:
all:
output: conda
conda_dir: output/actual
matrix:
cuda: ["11.5", "11.8"]
includes:
- cudatoolkit
channels:
- rapidsai
- conda-forge
dependencies:
cudatoolkit:
specific:
- output_types: conda
matrices:
- matrix:
cuda: "11.5"
packages:
- cudatoolkit=11.5
- matrix:
cuda: "11.*"
packages:
- cudatoolkit=11.*
- matrix:
packages:
- cudatoolkit
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# This file is generated by `rapids-dependency-file-generator`.
# To make changes, edit ../../dependencies.yaml and run `rapids-dependency-file-generator`.
channels:
- rapidsai
- conda-forge
dependencies:
- cudatoolkit=11.5
name: all_cuda-115
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
# This file is generated by `rapids-dependency-file-generator`.
# To make changes, edit ../../dependencies.yaml and run `rapids-dependency-file-generator`.
channels:
- rapidsai
- conda-forge
dependencies:
- cudatoolkit=11.*
name: all_cuda-118
7 changes: 4 additions & 3 deletions tests/test_examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,12 @@

# Erroneous examples raise runtime errors from the generator.
_erroneous_examples = [
"duplicate-specific-matrix-entries",
"no-specific-match",
"pyproject_matrix_multi",
"pyproject_bad_key",
"pyproject-no-extras",
"requirements-pip-dict"
"pyproject_bad_key",
"pyproject_matrix_multi",
"requirements-pip-dict",
]
EXAMPLE_FILES = [
pth
Expand Down
Loading