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

Create a version 4.0 format which doesn't include the expectations section #224

Closed
evansd opened this issue Nov 7, 2023 · 3 comments
Closed
Assignees

Comments

@evansd
Copy link
Contributor

evansd commented Nov 7, 2023

The expectations configuration e.g.

expectations:
  population_size: 1000

is no longer used by ehrQL, and therefore we should get rid of it.

However it is currently required by version 3.0 of the project.yaml format:

# The version of `project.yaml` where each feature was introduced
FEATURE_FLAGS_BY_VERSION = {
"UNIQUE_OUTPUT_PATH": 2,
"EXPECTATIONS_POPULATION": 3,
}

pipeline/pipeline/models.py

Lines 186 to 211 in 8c706d6

def validate_expectations_per_version(cls, values: RawPipeline) -> RawPipeline:
"""Ensure the expectations key exists for version 3 onwards"""
try:
version = float(values["version"])
except (KeyError, TypeError, ValueError):
# this is handled in the validate_version_exists and
# validate_version_value validators
return values
feat = get_feature_flags_for_version(version)
if not feat.EXPECTATIONS_POPULATION:
# set the default here because pydantic doesn't seem to set it
# otherwise
values["expectations"] = {"population_size": 1000}
return values
if "expectations" not in values:
raise ValueError("Project must include `expectations` section")
if "population_size" not in values["expectations"]:
raise ValueError(
"Project `expectations` section must include `population_size` section",
)
return values

Version 4.0 should reject this section.

Once we have done this we should update the research-template accordingly.

See also:

@lucyb
Copy link
Contributor

lucyb commented Nov 8, 2023

After speaking with Peter, this doesn't sound urgent and could be done by the upcoming RAP Team rather than the current Pipeline Team. Please let me know if I've missed anything.

@inglesp
Copy link
Contributor

inglesp commented Sep 30, 2024

When we introduce version 4, we'll also want to raise a validation error if there are any cohort-extractor actions in a project.yaml.

Additionally, we should:

@alarthast
Copy link
Contributor

alarthast commented Oct 4, 2024

The steps are:

then, do the tasks listed in Peter's comment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants