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: HiFa v1.1.0 schema development #1978

Open
wants to merge 40 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
40 commits
Select commit Hold shift + click to select a range
4934ed9
update validator to latest draft
kratsg Sep 1, 2022
2569988
add v1.0.1
kratsg Sep 1, 2022
bb7b416
move load_schema for defs.json as referrer document to ensure we alwa…
kratsg Sep 1, 2022
3dd5766
add version to typing
kratsg Sep 1, 2022
81c5816
add warning when version is wrong
kratsg Sep 1, 2022
9a67f9f
add utility for upgrading specifications
kratsg Sep 1, 2022
c9da4e3
add importlib_resources
kratsg Sep 1, 2022
bc8de57
add in new things into typing
kratsg Sep 1, 2022
475c08e
add typing into schema
kratsg Sep 1, 2022
a4358c6
type schema
kratsg Sep 1, 2022
8d67cc4
fix it up
kratsg Sep 1, 2022
f30da27
fix typo
kratsg Sep 2, 2022
a2f4899
add upgrade CLI
kratsg Sep 2, 2022
41a0b23
expose api
kratsg Sep 2, 2022
58c79f0
add tests for the upgrade functionality in python
kratsg Sep 2, 2022
7b60642
don't lock patchset and workspace together
kratsg Dec 6, 2022
58fe1b7
fix some tests
kratsg Dec 6, 2022
9b8ea83
fix broken tests
kratsg Dec 6, 2022
691a3b5
fix up more tests
kratsg Dec 6, 2022
26a5f32
fix pre-commit
kratsg Dec 6, 2022
b75ab60
add typing_extensions
kratsg Dec 6, 2022
b15850c
add 1.1.0 and keep latest schema version at 1.0.0
kratsg Dec 7, 2022
b91ee76
fix vers
kratsg Dec 7, 2022
c89eff0
fix up test
kratsg Dec 7, 2022
8f10cd8
fix typing
kratsg Dec 7, 2022
b6edb60
switch to class-based so we can always upgrade between diff versions
kratsg Dec 7, 2022
f854c23
fix up tests for now
kratsg Dec 7, 2022
4f84a65
fix up coverage
kratsg Dec 7, 2022
06f13a9
update version to 1.1.0
kratsg Dec 7, 2022
4563ae3
update draft
kratsg Dec 7, 2022
3bb4d0e
Revert "update draft"
kratsg Dec 7, 2022
fcfab82
downgrade validator to maintain behavior for correct draft for v1.0.0…
kratsg Dec 7, 2022
862aabb
switch to draft/2020-12 default
kratsg Dec 7, 2022
6249ffa
test upgrade 1.0.1 to itself
kratsg Dec 8, 2022
f05cb95
fix and use context manager
kratsg Feb 2, 2023
531e0ba
fix coverage
kratsg Feb 2, 2023
bc7fffa
initial fix up of typehints
kratsg Dec 7, 2023
e66d252
fix up
kratsg Dec 7, 2023
f1df51f
Merge branch 'main' into feat/updateJSONSchemaDraft
kratsg Dec 7, 2023
372c8d2
don't change draft version for 1.0.0
kratsg Dec 7, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ repos:
name: mypy with Python 3.8
files: src
additional_dependencies:
['numpy', 'types-tqdm', 'click', 'types-jsonpatch', 'types-pyyaml', 'types-jsonschema', 'importlib_metadata', 'packaging']
['numpy', 'types-tqdm', 'click', 'types-jsonpatch', 'types-pyyaml', 'types-jsonschema', 'importlib_metadata', 'packaging', 'importlib_resources']
args: ["--python-version=3.8"]
- <<: *mypy
name: mypy with Python 3.11
Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ classifiers = [
dependencies = [
"click>=8.0.0", # for console scripts
"importlib_resources>=1.4.0; python_version < '3.9'", # for resources in schema
"typing_extensions; python_version < '3.11'", # for typing
"jsonpatch>=1.15",
"jsonschema>=4.15.0", # for utils
"pyyaml>=5.1", # for parsing CLI equal-delimited options
Expand Down Expand Up @@ -264,7 +265,6 @@ module = [
'pyhf.modifiers.*',
'pyhf.exceptions.*',
'pyhf.parameters.*',
'pyhf.schema.*',
'pyhf.writexml',
'pyhf.workspace',
'pyhf.patchset',
Expand Down
4 changes: 3 additions & 1 deletion src/pyhf/cli/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import click

from pyhf import __version__
from pyhf.cli import rootio, spec, infer, patchset, complete
from pyhf.cli import rootio, spec, infer, patchset, complete, upgrade
from pyhf.contrib import cli as contrib
from pyhf import utils

Expand Down Expand Up @@ -56,3 +56,5 @@ def pyhf():
pyhf.add_command(complete.cli)

pyhf.add_command(contrib.cli)

pyhf.add_command(upgrade.cli)
72 changes: 72 additions & 0 deletions src/pyhf/cli/upgrade.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
"""The pyhf upgrade CLI subcommand."""
import logging

import click
import json

from pyhf.schema.upgrader import upgrade

log = logging.getLogger(__name__)


@click.group(name='upgrade')
def cli():
"""Operations for upgrading specifications."""


@cli.command()
@click.argument('workspace', default='-')
@click.option(
'--version',
help='The version to upgrade to',
default=None,
)
@click.option(
'--output-file',
help='The location of the output json file. If not specified, prints to screen.',
default=None,
)
def workspace(workspace, version, output_file):
"""
Upgrade a HistFactory JSON workspace.
"""
with click.open_file(workspace, 'r', encoding="utf-8") as specstream:
spec = json.load(specstream)

ws = upgrade(to_version=version).workspace(spec)

if output_file is None:
click.echo(json.dumps(ws, indent=4, sort_keys=True))
else:
with open(output_file, 'w+', encoding="utf-8") as out_file:
json.dump(ws, out_file, indent=4, sort_keys=True)
log.debug(f"Written to {output_file:s}")


@cli.command()
@click.argument('patchset', default='-')
@click.option(
'--version',
help='The version to upgrade to',
default=None,
)
@click.option(
'--output-file',
help='The location of the output json file. If not specified, prints to screen.',
default=None,
)
def patchset(patchset, version, output_file):
"""
Upgrade a pyhf JSON PatchSet.
"""
with click.open_file(patchset, 'r', encoding="utf-8") as specstream:
spec = json.load(specstream)

ps = upgrade(to_version=version).patchset(spec)

if output_file is None:
click.echo(json.dumps(ps, indent=4, sort_keys=True))
else:
with open(output_file, 'w+', encoding="utf-8") as out_file:
json.dump(ps, out_file, indent=4, sort_keys=True)
log.debug(f"Written to {output_file:s}")
2 changes: 1 addition & 1 deletion src/pyhf/readxml.py
Original file line number Diff line number Diff line change
Expand Up @@ -473,7 +473,7 @@ def parse(
'measurements': measurements,
'channels': channels,
'observations': observations,
'version': schema.version, # type: ignore[typeddict-unknown-key]
'version': schema.versions['workspace.json'], # type: ignore[attr-defined]
}
try:
schema.validate(result, 'workspace.json')
Expand Down
24 changes: 15 additions & 9 deletions src/pyhf/schema/__init__.py
Original file line number Diff line number Diff line change
@@ -1,25 +1,31 @@
"""
See :class:`~pyhf.schema.Schema` for documentation.
"""
import pathlib
from __future__ import annotations
import sys
from typing import Any
from pyhf.schema.loader import load_schema
from pyhf.schema.validator import validate
from pyhf.schema import variables
from pyhf.typing import Self, SchemaVersion, PathOrStr, Traversable
from pyhf.schema.upgrader import upgrade

from pathlib import Path

__all__ = [
"load_schema",
"validate",
"path",
"version",
"upgrade",
]


def __dir__():
def __dir__() -> list[str]:
return __all__


class Schema(sys.modules[__name__].__class__):
class Schema(sys.modules[__name__].__class__): # type: ignore[misc]
"""
A module-level wrapper around :mod:`pyhf.schema` which will provide additional functionality for interacting with schemas.

Expand Down Expand Up @@ -61,7 +67,7 @@ class Schema(sys.modules[__name__].__class__):

"""

def __call__(self, new_path: pathlib.Path):
def __call__(self, new_path: PathOrStr) -> Self:
"""
Change the local search path for finding schemas locally.

Expand All @@ -71,15 +77,15 @@ def __call__(self, new_path: pathlib.Path):
Returns:
self (pyhf.schema.Schema): Returns itself (for contextlib management)
"""
self.orig_path, variables.schemas = variables.schemas, new_path
self.orig_path, variables.schemas = variables.schemas, Path(new_path)
self.orig_cache = dict(variables.SCHEMA_CACHE)
variables.SCHEMA_CACHE.clear()
return self

def __enter__(self):
def __enter__(self) -> None:
pass

def __exit__(self, *args, **kwargs):
def __exit__(self, *args: Any, **kwargs: Any) -> None:
"""
Reset the local search path for finding schemas locally.

Expand All @@ -90,14 +96,14 @@ def __exit__(self, *args, **kwargs):
variables.SCHEMA_CACHE = self.orig_cache

@property
def path(self):
def path(self) -> Traversable | Path:
"""
The local path for schemas.
"""
return variables.schemas

@property
def version(self):
def versions(self) -> dict[str, SchemaVersion]:
"""
The default version used for finding schemas.
"""
Expand Down
9 changes: 2 additions & 7 deletions src/pyhf/schema/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
import json
import pyhf.exceptions
from pyhf.schema import variables
from pyhf.typing import Schema

# importlib.resources.as_file wasn't added until Python 3.9
# c.f. https://docs.python.org/3.9/library/importlib.html#importlib.resources.as_file
Expand All @@ -12,7 +13,7 @@
import importlib_resources as resources


def load_schema(schema_id: str):
def load_schema(schema_id: str) -> Schema:
"""
Get a schema by relative path from cache, or load it into the cache and return.

Expand Down Expand Up @@ -54,9 +55,3 @@ def load_schema(schema_id: str):
schema = json.load(json_schema)
variables.SCHEMA_CACHE[schema['$id']] = schema
return variables.SCHEMA_CACHE[schema['$id']]


# pre-populate the cache to avoid network access
kratsg marked this conversation as resolved.
Show resolved Hide resolved
# on first validation in standard usage
# (not in pyhf.schema.variables to avoid circular imports)
load_schema(f'{variables.SCHEMA_VERSION}/defs.json')
61 changes: 61 additions & 0 deletions src/pyhf/schema/upgrader.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
from __future__ import annotations

from pyhf.typing import Workspace, PatchSet, SchemaVersion, UpgradeProtocol
import copy


class Upgrade_1_0_1:
"""
Used for testing functionality of upgrade.
"""

version: SchemaVersion = '1.0.1'
Comment on lines +7 to +12
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
class Upgrade_1_0_1:
"""
Used for testing functionality of upgrade.
"""
version: SchemaVersion = '1.0.1'
class Upgrade(version: SchemaVersion = "1.0.1"):
"""
Used for testing functionality of upgrade.
"""

Maybe I'm missing something about what the purpose of upgrader is, but is there a reason why we need to make this class schema v1.0.1 hardcoded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need an Upgrade_X_Y_Z class for every single version we plan to support upgrading to. When we add a new version of the spec, we have to have a new upgrade class that knows how to upgrade from an old version to the version the class represents.

Copy link
Member

Choose a reason for hiding this comment

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

@kratsg Ah okay, so the real reason is that we can't deterministically know what version we're upgrading from in advance?

Do we have a plan for how to handle upgrading multiple versions in one go? e.g. Let's say we have v1.0.0, v1.1.0, v1.2.0, and v2.0.0. If at this point someone comes along with a v1.0.0schema and wants to move fromv1.0.0tov2.0.0` what's the plan for doing this?

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 have ideas on how to do this with python's mro


@classmethod
def workspace(cls, spec: Workspace) -> Workspace:
"""
Upgrade the provided workspace specification.

Args:
spec (dict): The specification to validate.
schema_name (str): The name of the schema to upgrade.

Returns:
upgraded_spec (dict): Upgraded workspace specification.

Raises:
pyhf.exceptions.InvalidSpecification: the specification is invalid
"""

new_spec = copy.deepcopy(spec)
if spec['version'] == '1.0.0':
new_spec['version'] = cls.version
return new_spec

@classmethod
def patchset(cls, spec: PatchSet) -> PatchSet:
"""
Upgrade the provided patchset specification.

Args:
spec (dict): The specification to validate.
schema_name (str): The name of the schema to upgrade.

Returns:
upgraded_spec (dict): Upgraded patchset specification.

Raises:
pyhf.exceptions.InvalidSpecification: the specification is invalid
"""

new_spec = copy.deepcopy(spec)
if spec['version'] == '1.0.0':
new_spec['version'] = cls.version
return new_spec


def upgrade(*, to_version: SchemaVersion | None = None) -> type[UpgradeProtocol]:
if to_version is None or to_version == '1.0.1':
return Upgrade_1_0_1
Comment on lines +57 to +59
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
def upgrade(*, to_version: SchemaVersion | None = None) -> type[UpgradeProtocol]:
if to_version is None or to_version == '1.0.1':
return Upgrade_1_0_1
def upgrade(*, to_version: SchemaVersion | None = None) -> type[UpgradeProtocol]:
if to_version is None or to_version == "1.0.1":
return Upgrade("1.0.1")

Follow up to the above.


raise ValueError(f'{to_version} is not a valid version to upgrade to.')