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.

Sign up for GitHub

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

Jump to bottom

feat: Generate meta.yaml dependencies #45

Closed
wants to merge 6 commits into from
Closed

feat: Generate meta.yaml dependencies #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5df4e2a
Add constants for meta.yaml.
vyasr Nov 24, 2022
cf0d6a1
Add comments
vyasr Mar 23, 2023
5c8f778
Update schema
vyasr Mar 24, 2023
79b13e4
Add support for generating separate dependency files
vyasr Mar 28, 2023
5b9aa8c
Temporarily remove not working schema validation
vyasr Mar 28, 2023
1196647
Cleanup
vyasr Mar 28, 2023
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
2 changes: 2 additions & 0 deletions src/rapids_dependency_file_generator/constants.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

class OutputTypes(Enum):
CONDA = "conda"
CONDA_META = "conda_meta"
REQUIREMENTS = "requirements"
PYPROJECT = "pyproject"
NONE = "none"
Expand All @@ -22,6 +23,7 @@ def __str__(self):
]

default_conda_dir = "conda/environments"
default_conda_meta_dir = "conda/recipes/"
Comment on lines 25 to +26
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why to use a trailing backslash here, but not for the other dirs?

default_requirements_dir = "python"
default_pyproject_dir = "python"
default_dependency_file_path = "dependencies.yaml"
33 changes: 31 additions & 2 deletions src/rapids_dependency_file_generator/rapids_dependency_file_generator.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
cli_name,
default_channels,
default_conda_dir,
default_conda_meta_dir,
default_pyproject_dir,
default_requirements_dir,
)
Expand Down Expand Up @@ -55,6 +56,8 @@ def dedupe(dependencies):
"""
deduped = sorted({dep for dep in dependencies if not isinstance(dep, dict)})
dict_deps = defaultdict(list)
# The purpose of the outer loop is to support nested dependency lists such as the
# `pip:` list. If multiple are present, they must be internally deduped as well.
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated?

for dep in filter(lambda dep: isinstance(dep, dict), dependencies):
for key, values in dep.items():
dict_deps[key].extend(values)
Expand Down Expand Up @@ -83,6 +86,10 @@ def grid(gridspec):
Iterable[dict]
Each yielded value is a dictionary containing one of the unique
combinations of parameter values from `gridspec`.

Notes
-----
An empty `gridspec` dict will result in an empty dict as the single yielded value.
Comment on lines +89 to +92
Copy link
Contributor

Choose a reason for hiding this comment

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

Unrelated?

"""
for values in itertools.product(*gridspec.values()):
yield dict(zip(gridspec.keys(), values))
Expand Down Expand Up @@ -131,6 +138,12 @@ def make_dependency_file(
"dependencies": dependencies,
}
)
elif file_type == str(OutputTypes.CONDA_META):
file_contents += yaml.dump(
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we assume that the header will be valid for all generated file types?

{
"dependencies": dependencies,
Copy link
Contributor

Choose a reason for hiding this comment

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

The "dependencies" section of a conda-meta file is called "requirements". What am I missing here?

}
)
elif file_type == str(OutputTypes.REQUIREMENTS):
file_contents += "\n".join(dependencies) + "\n"
elif file_type == str(OutputTypes.PYPROJECT):
Expand Down Expand Up @@ -208,7 +221,7 @@ def get_requested_output_types(output):
return output


def get_filename(file_type, file_key, matrix_combo):
def get_filename(file_type, file_key, matrix_combo, extras=None):
"""Get the name of the file to which to write a generated dependency set.

The file name will be composed of the following components, each determined
Expand All @@ -229,6 +242,8 @@ def get_filename(file_type, file_key, matrix_combo):
matrix_combo : dict
A mapping of key-value pairs corresponding to the
[files.$FILENAME.matrix] entry in dependencies.yaml.
extras : dict
Any extra information provided for generating this filename.

Returns
-------
Expand All @@ -240,6 +255,18 @@ def get_filename(file_type, file_key, matrix_combo):
file_name_prefix = file_key
if file_type == str(OutputTypes.CONDA):
file_ext = ".yaml"
elif file_type == str(OutputTypes.CONDA_META):
file_ext = ".yaml"
file_name_prefix = "_".join(
filter(
None,
[
"meta_dependencies",
extras.get("output", ""),
extras.get("section", ""),
Comment on lines +265 to +266
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
extras.get("output", ""),
extras.get("section", ""),
extras.get("output"),
extras.get("section"),

Since you are filtering on None anyways.

],
)
)
elif file_type == str(OutputTypes.REQUIREMENTS):
file_ext = ".txt"
file_type_prefix = "requirements"
Expand Down Expand Up @@ -282,6 +309,8 @@ def get_output_dir(file_type, config_file_path, file_config):
path = [os.path.dirname(config_file_path)]
if file_type == str(OutputTypes.CONDA):
path.append(file_config.get("conda_dir", default_conda_dir))
elif file_type == str(OutputTypes.CONDA_META):
path.append(file_config.get("conda_meta_dir", default_conda_meta_dir))
elif file_type == str(OutputTypes.REQUIREMENTS):
path.append(file_config.get("requirements_dir", default_requirements_dir))
elif file_type == str(OutputTypes.PYPROJECT):
Expand Down Expand Up @@ -425,7 +454,7 @@ def make_dependency_files(parsed_config, config_file_path, to_stdout):
)

# Dedupe deps and print / write to filesystem
full_file_name = get_filename(file_type, file_key, matrix_combo)
full_file_name = get_filename(file_type, file_key, matrix_combo, extras)
deduped_deps = dedupe(dependencies)

output_dir = (
Expand Down
21 changes: 8 additions & 13 deletions src/rapids_dependency_file_generator/schema.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
"matrix": {"$ref": "#/$defs/matrix"},
"requirements_dir": {"type": "string"},
"conda_dir": {"type": "string"},
"conda_meta_dir": {"type": "string"},
"pyproject_dir": {"type": "string"}
},
"additionalProperties": false,
Expand Down Expand Up @@ -118,7 +119,7 @@
"items": {"$ref": "#/$defs/matrix-matcher"}
},
"output-types": {
"enum": ["conda", "requirements", "pyproject"]
"enum": ["conda", "conda_meta", "requirements", "pyproject"]
},
"output-types-array": {
"type": "array",
Expand Down Expand Up @@ -162,22 +163,16 @@
"extras": {
"type": "object",
"properties": {
"section": {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does the keyword "section" originate from the conda docs? I am wondering whether it would make sense to better namespace our "extras", something like "conda-meta-section" instead of just "section".

"type": "string",
"enum": ["build", "host", "run"]
},
"table": {
"type": "string",
"enum": ["build-system", "project", "project.optional-dependencies"]
},
"key": {"type": "string"}
},
"if": {
"properties": { "table": { "const": "project.optional-dependencies" } }
},
"then": {
"required": ["key"]
},
"else": {
"not": {
"required": ["key"]
}
Comment on lines -171 to -180
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I was having trouble getting this conditional to behave correctly. According to the official JSON Schema docs it should have been sufficient to add a "required": ["table"] to the if statement, but no matter what I did the validator insisted that the key property was required when only the section key was present. I'll need to revisit this but didn't want to block the rest of the review on a schema issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

CC @csadorf if you have thoughts here.

Copy link
Contributor

Choose a reason for hiding this comment

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

So when you simply added a "required": ["table"] to the "if" block and removed the "then" and "else" blocks it would always require it? That seems indeed very odd. Can you provide some valid/invalid examples that we can test against?

"key": {"type": "string"},
"additionalProperties": false
},
"additionalProperties": false
}
Expand Down