From 6f8303ebad9b0fb8132d5f8517d5d13b02bb8e9c Mon Sep 17 00:00:00 2001 From: Mihir Patel Date: Thu, 28 Jul 2022 11:18:49 -0700 Subject: [PATCH] Revert "JSON Schema Support (#123)" This reverts commit 8d1880e9825c2e6a1fc4f574be421cdd4a620539. --- meta.yaml | 1 - setup.py | 4 +- tests/test_json_schema.py | 226 ---------------------------- tests/yahp_fixtures.py | 67 +-------- yahp/create_object/argparse.py | 11 +- yahp/create_object/create_object.py | 20 +-- yahp/hparams.py | 75 +-------- yahp/utils/json_schema_helpers.py | 91 ----------- 8 files changed, 14 insertions(+), 481 deletions(-) delete mode 100644 tests/test_json_schema.py delete mode 100644 yahp/utils/json_schema_helpers.py diff --git a/meta.yaml b/meta.yaml index 88b0509..49de755 100644 --- a/meta.yaml +++ b/meta.yaml @@ -29,7 +29,6 @@ requirements: - python >=3.7 - pyyaml >=5.4.1 - ruamel.yaml >=0.17.10 - - jsonschema >=4.7.2, <4.8 test: requires: diff --git a/setup.py b/setup.py index be3b0a3..2556280 100755 --- a/setup.py +++ b/setup.py @@ -8,9 +8,7 @@ exec(open('yahp/version.py', 'r', encoding='utf-8').read()) -install_requires = [ - 'PyYAML>=5.4.1', 'ruamel.yaml>=0.17.10', 'docstring_parser>=0.14.1,<=0.15', 'jsonschema>=4.7.2,<4.8' -] +install_requires = ['PyYAML>=5.4.1', 'ruamel.yaml>=0.17.10', 'docstring_parser>=0.14.1,<=0.15'] extra_deps = {} diff --git a/tests/test_json_schema.py b/tests/test_json_schema.py deleted file mode 100644 index 5601f49..0000000 --- a/tests/test_json_schema.py +++ /dev/null @@ -1,226 +0,0 @@ -import contextlib -import json -import os -import pathlib -import textwrap -from typing import Type - -import pytest -import yaml -from jsonschema import ValidationError - -from tests.yahp_fixtures import (BearsHparams, ChoiceHparamParent, KitchenSinkHparams, PrimitiveHparam, - ShavingBearsHparam) -from yahp.hparams import Hparams - - -@pytest.mark.parametrize('hparam_class,success,data', [ - [ - PrimitiveHparam, True, - textwrap.dedent(""" - --- - intfield: 1 - strfield: hello - floatfield: 0.5 - boolfield: true - enumintfield: ONE - enumstringfield: mosaic - jsonfield: - empty_item: {} - random_item: 1 - random_item2: two - random_item3: true - random_item4: 0.1 - random_subdict: - random_subdict_item5: 12 - random_subdict_item6: 1337 - random_list: - - 1 - - 3 - - 99 - random_list_of_dict: - - sub_dict: 12 - sub_dict_item: 1 - - sub_dict2: 14 - sub_dict_item: 43 - """) - ], - [ - PrimitiveHparam, False, - textwrap.dedent(""" - --- - strfield: hello - floatfield: 0.5 - boolfield: true - enumintfield: ONE - enumstringfield: mosaic - jsonfield: - empty_item: {} - random_item: 1 - random_item2: two - random_item3: true - random_item4: 0.1 - random_subdict: - random_subdict_item5: 12 - random_subdict_item6: 1337 - random_list: - - 1 - - 3 - - 99 - random_list_of_dict: - - sub_dict: 12 - sub_dict_item: 1 - - sub_dict2: 14 - sub_dict_item: 43 - """) - ], - [ - KitchenSinkHparams, - True, - textwrap.dedent(""" - --- - required_int_field: 1 - nullable_required_int_field: - required_bool_field: False - nullable_required_bool_field: - required_enum_field_list: - - RED - - red - required_enum_field_with_default: green - nullable_required_enum_field: None - nullable_required_enum_field: - required_union_bool_str_field: hi - nullable_required_union_bool_str_field: - required_int_list_field: - - 1 - - 2 - - 42 - nullable_required_list_int_field: - - 24 - nullable_required_list_union_bool_str_field: - required_list_union_bool_str_field: - - True - - hi - - False - - bye - required_subhparams_field: { default_false: False, default_true: True } - nullable_required_subhparams_field: - required_subhparams_field_list: - - { default_false: False } - - {} - nullable_required_subhparams_field_list: - required_choice: - one: - commonfield: True - intfield: 3 - nullable_required_choice: - required_choice_list: [] - nullable_required_choice_list: - optional_float_field_default_1: 1.1 - """), - ], - [ChoiceHparamParent, True, - textwrap.dedent(""" - --- - commonfield: True - """)], - [ChoiceHparamParent, False, - textwrap.dedent(""" - --- - commonfield: 1 - """)], - [ - ShavingBearsHparam, True, - textwrap.dedent(""" - --- - parameters: - random_field: - shaved_bears: - first_action: "Procure bears" - last_action: "Release bears into wild with stylish new haircuts" - other_random_field: "cool" - """) - ], - [BearsHparams, True, textwrap.dedent(""" - --- - bears: - """)], - [ - BearsHparams, True, - textwrap.dedent(""" - --- - bears: - - shaved_bears: - first_action: "Procure bears" - last_action: "Release bears into wild with stylish new haircuts" - """) - ], - [ - BearsHparams, True, - textwrap.dedent(""" - --- - bears: - - unshaved_bears: - second_action: "Procure bears" - third_action: "Release bears into wild with stylish new haircuts" - """) - ], - [ - BearsHparams, True, - textwrap.dedent(""" - --- - bears: - - shaved_bears: - first_action: "Procure bears" - last_action: "Release bears into wild with stylish new haircuts" - - unshaved_bears: - second_action: "Procure bears" - third_action: "Release bears into wild with stylish new haircuts" - """) - ], -]) -def test_validate_json_schema_from_strings(hparam_class: Type[Hparams], success: bool, data: str): - with contextlib.nullcontext() if success else pytest.raises(ValidationError): - hparam_class.validate_yaml(data=yaml.safe_load(data)) - - -@pytest.mark.parametrize('hparam_class,success,file', [ - [ShavingBearsHparam, True, - os.path.join(os.path.dirname(__file__), 'inheritance/shaving_bears.yaml')], -]) -def test_validate_json_schema_from_file(hparam_class: Type[Hparams], success: bool, file: str): - with contextlib.nullcontext() if success else pytest.raises(ValidationError): - hparam_class.validate_yaml(f=file) - - -@pytest.mark.parametrize('hparam_class', [ - ShavingBearsHparam, - ChoiceHparamParent, - PrimitiveHparam, - KitchenSinkHparams, - BearsHparams, -]) -def test_write_and_read_json_schema_from_name(hparam_class: Type[Hparams], tmp_path: pathlib.Path): - file = os.path.join(tmp_path, 'schema.json') - hparam_class.dump_jsonschema(file) - with open(file) as f: - loaded_schema = json.load(f) - generated_schema = hparam_class.get_json_schema() - assert loaded_schema == generated_schema - - -@pytest.mark.parametrize('hparam_class', [ - ShavingBearsHparam, - ChoiceHparamParent, - PrimitiveHparam, - KitchenSinkHparams, - BearsHparams, -]) -def test_write_and_read_json_schema_from_file(hparam_class: Type[Hparams], tmp_path: pathlib.Path): - file = os.path.join(tmp_path, 'schema.json') - with open(file, 'w') as f: - hparam_class.dump_jsonschema(f) - with open(file) as f: - loaded_schema = json.load(f) - generated_schema = hparam_class.get_json_schema() - assert loaded_schema == generated_schema diff --git a/tests/yahp_fixtures.py b/tests/yahp_fixtures.py index 65da617..2765816 100755 --- a/tests/yahp_fixtures.py +++ b/tests/yahp_fixtures.py @@ -4,13 +4,12 @@ import textwrap from dataclasses import dataclass from enum import Enum, IntEnum -from typing import Any, Dict, List, NamedTuple, Optional, Type, Union, cast +from typing import Any, Dict, List, NamedTuple, Optional, Union, cast import pytest import yaml import yahp as hp -from yahp.hparams import Hparams from yahp.types import JSON @@ -119,7 +118,7 @@ def primitive_yaml_input(hparams_tempdir: pathlib.Path) -> YamlInput: floatfield: 0.5 boolfield: true enumintfield: ONE - enumstringfield: mosaic + enumstringfield: pytorch_lightning jsonfield: empty_item: {} random_item: 1 @@ -592,65 +591,3 @@ def optional_required_yaml_input(hparams_tempdir) -> YamlInput: return generate_named_tuple_from_data(hparams_tempdir=hparams_tempdir, input_data=data, filepath='optional_required.yaml') - - -@dataclass -class ShavedBearsHparam(hp.Hparams): - first_action: str = hp.required(doc='str field') - last_action: str = hp.required(doc='str field') - - def validate(self): - assert isinstance(self.first_action, str) - assert isinstance(self.last_action, str) - super().validate() - - -@dataclass -class UnshavedBearsHparam(hp.Hparams): - second_action: str = hp.required(doc='str field') - third_action: str = hp.required(doc='str field') - - def validate(self): - assert isinstance(self.second_action, str) - assert isinstance(self.third_action, str) - super().validate() - - -@dataclass -class ParametersHparam(hp.Hparams): - random_field: Optional[int] = hp.required(doc='int field') - shaved_bears: ShavedBearsHparam = hp.required(doc='ShavedBears Hparams') - other_random_field: str = hp.required(doc='str field') - - def validate(self): - assert isinstance(self.random_field, int) - assert isinstance(self.shaved_bears, ParametersHparam) - self.shaved_bears.validate() - assert isinstance(self.other_random_field, str) - super().validate() - - -@dataclass -class ShavingBearsHparam(hp.Hparams): - parameters: ParametersHparam = hp.required(doc='Parameters Hparams') - - def validate(self): - assert isinstance(self.parameters, ParametersHparam) - self.parameters.validate() - super().validate() - - -bears_registry: Dict[str, Type[hp.Hparams]] = { - 'shaved_bears': ShavedBearsHparam, - 'unshaved_bears': UnshavedBearsHparam, -} - - -@dataclass -class BearsHparams(hp.Hparams): - - hparams_registry = { - 'bears': bears_registry, - } - - bears: Optional[List[Hparams]] = hp.required(doc='bear field') diff --git a/yahp/create_object/argparse.py b/yahp/create_object/argparse.py index 80c4019..8c9a96b 100644 --- a/yahp/create_object/argparse.py +++ b/yahp/create_object/argparse.py @@ -117,10 +117,10 @@ def get_hparams_file_from_cli( cli_args: List[str], argparse_name_registry: ArgparseNameRegistry, argument_parsers: List[argparse.ArgumentParser], -) -> Tuple[Optional[str], Optional[str], Optional[str]]: +) -> Tuple[Optional[str], Optional[str]]: parser = argparse.ArgumentParser(add_help=False) argument_parsers.append(parser) - argparse_name_registry.reserve('f', 'file', 'd', 'dump', 'validate') + argparse_name_registry.reserve('f', 'file', 'd', 'dump') parser.add_argument('-f', '--file', type=str, @@ -139,13 +139,8 @@ def get_hparams_file_from_cli( metavar='stdout', help='Dump the resulting Hparams to the specified YAML file (defaults to `stdout`) and exit.', ) - parser.add_argument( - '--validate', - action='store_true', - help='Whether to validate YAML against Hparams.', - ) parsed_args, cli_args[:] = parser.parse_known_args(cli_args) - return parsed_args.file, parsed_args.dump, parsed_args.validate + return parsed_args.file, parsed_args.dump def get_commented_map_options_from_cli( diff --git a/yahp/create_object/create_object.py b/yahp/create_object/create_object.py index 84e4809..8e4c5db 100755 --- a/yahp/create_object/create_object.py +++ b/yahp/create_object/create_object.py @@ -645,7 +645,7 @@ def _get_hparams( ) if cm_options is not None: output_file, interactive, add_docs = cm_options - print(f'Generating a template for {constructor.__name__}...') + print(f'Generating a template for {constructor.__name__}') cls = ensure_hparams_cls(constructor) if output_file == 'stdout': cls.dump(add_docs=add_docs, interactive=interactive, output=sys.stdout) @@ -655,21 +655,13 @@ def _get_hparams( with open(output_file, 'x') as f: cls.dump(add_docs=add_docs, interactive=interactive, output=f) # exit so we don't attempt to parse and instantiate if generate template is passed - print('\nFinished') - sys.exit(0) - - cli_f, output_f, validate = get_hparams_file_from_cli(cli_args=remaining_cli_args, - argparse_name_registry=argparse_name_registry, - argument_parsers=argparsers) - # Validate was specified, so only validate instead of instantiating - if validate: - print(f'Validating YAML against {constructor.__name__}...') - cls = ensure_hparams_cls(constructor) - cls.validate_yaml(f=cli_f) - # exit so we don't attempt to parse and instantiate - print('\nSuccessfully validated YAML!') + print() + print('Finished') sys.exit(0) + cli_f, output_f = get_hparams_file_from_cli(cli_args=remaining_cli_args, + argparse_name_registry=argparse_name_registry, + argument_parsers=argparsers) if cli_f is not None: if f is not None: raise ValueError('File cannot be specified via both function arguments and the CLI') diff --git a/yahp/hparams.py b/yahp/hparams.py index 675c12b..ae3d60a 100755 --- a/yahp/hparams.py +++ b/yahp/hparams.py @@ -3,7 +3,6 @@ from __future__ import annotations import argparse -import json import logging import pathlib import textwrap @@ -11,16 +10,13 @@ from abc import ABC from dataclasses import dataclass, fields from enum import Enum -from io import StringIO, TextIOWrapper -from typing import (TYPE_CHECKING, Any, Callable, Dict, List, Optional, TextIO, Type, TypeVar, Union, cast, - get_type_hints) +from io import StringIO +from typing import TYPE_CHECKING, Any, Callable, Dict, List, Optional, TextIO, Type, TypeVar, Union, cast -import jsonschema import yaml from yahp.utils import type_helpers from yahp.utils.iter_helpers import list_to_deduplicated_dict -from yahp.utils.json_schema_helpers import get_registry_json_schema, get_type_json_schema if TYPE_CHECKING: from yahp.types import JSON @@ -329,73 +325,6 @@ def validate(self): category=DeprecationWarning, ) - @classmethod - def get_json_schema(cls) -> Dict[str, Any]: - """Generates and returns a JSONSchema dictionary.""" - - res = { - 'type': 'object', - 'properties': {}, - 'additionalProperties': False, - } - class_type_hints = get_type_hints(cls) - for f in fields(cls): - if not f.init: - continue - - # Required field - if type_helpers.is_field_required(f): - if 'required' not in res.keys(): - res['required'] = [] - res['required'].append(f.name) - - hparams_type = type_helpers.HparamsType(class_type_hints[f.name]) - # Name is found in registry, set possible values as types in a union type - if cls.hparams_registry and f.name in cls.hparams_registry and len(cls.hparams_registry[f.name].keys()) > 0: - res['properties'][f.name] = get_registry_json_schema(hparams_type, cls.hparams_registry[f.name]) - else: - res['properties'][f.name] = get_type_json_schema(hparams_type) - res['properties'][f.name]['description'] = f.metadata['doc'] - - return res - - @classmethod - def dump_jsonschema(cls, f: Union[TextIO, str, pathlib.Path]): - """Dump the JSONSchema to ``f``.""" - if isinstance(f, TextIO) or isinstance(f, TextIOWrapper): - json.dump(cls.get_json_schema(), f) - else: - with open(f, 'w') as file: - json.dump(cls.get_json_schema(), file) - - @classmethod - def validate_yaml(cls: Type[THparams], - f: Union[str, None, TextIO, pathlib.PurePath] = None, - data: Optional[Dict[str, Any]] = None): - """Validate yaml against JSON schema. - - Args: - f (Union[str, None, TextIO, pathlib.PurePath], optional): - If specified, loads values and validates from a YAML file. Can be either a - filepath or file-like object. - Cannot be specified with ``data``. - data (Optional[str], optional): - If specified, validates YAML specified by string :class:`Hparams`. - Cannot be specified with ``f``. - """ - if f and data: - raise ValueError('File and data cannot both be specified.') - elif f: - if isinstance(f, TextIO) or isinstance(f, TextIOWrapper): - jsonschema.validate(yaml.safe_load(f), cls.get_json_schema()) - else: - with open(f) as file: - jsonschema.validate(yaml.safe_load(file), cls.get_json_schema()) - elif data: - jsonschema.validate(data, cls.get_json_schema()) - else: - raise ValueError('Neither file nor data were provided, so there is no YAML to validate.') - def __str__(self) -> str: yaml_str = self.to_yaml().strip() yaml_str = textwrap.indent(yaml_str, ' ') diff --git a/yahp/utils/json_schema_helpers.py b/yahp/utils/json_schema_helpers.py deleted file mode 100644 index af15276..0000000 --- a/yahp/utils/json_schema_helpers.py +++ /dev/null @@ -1,91 +0,0 @@ -import inspect -from enum import Enum -from typing import Any, Dict - -from yahp.utils import type_helpers - - -def get_registry_json_schema(f_type: type_helpers.HparamsType, registry: Dict[str, Any]): - """Convert type into corresponding JSON Schema. As the given name is in the `hparams_registry`, - create objects for each possible entry in the registry and treat as union type. - """ - res = {'anyOf': []} - for key, value in registry.items(): - res['anyOf'].append({ - 'type': 'object', - 'properties': { - key: get_type_json_schema(type_helpers.HparamsType(value)) - }, - 'additionalProperties': False, - }) - return _check_for_list_and_optional(f_type, res) - - -def get_type_json_schema(f_type: type_helpers.HparamsType): - """Convert type into corresponding JSON Schema. We first check for union types and recursively - handle each component. If a type is not union, we know it is a singleton type, so it must be - either a primitive, Enum, JSON, or Hparam-like. Dictionaries are treated as JSON types, and - list and optionals are handled in a post-processing step in `_check_for_list_and_optional`. - """ - # Import inside function to resolve circular dependencies - from yahp.auto_hparams import ensure_hparams_cls - - res = {} - - # Union - if len(f_type.types) > 1: - # Add all union types using anyOf - res = {'anyOf': []} - for union_type in f_type.types: - res['anyOf'].append(get_type_json_schema(type_helpers.HparamsType(union_type))) - # Primitive Types - elif f_type.type is str: - res = {'type': 'string'} - elif f_type.type is bool: - res = {'type': 'boolean'} - elif f_type.type is int: - res = {'type': 'integer'} - elif f_type.type is float: - res = {'type': 'number'} - # Enum - elif inspect.isclass(f_type.type) and issubclass(f_type.type, Enum): - # Enum attributes can either be specified lowercase or uppercase - member_names = [name.lower() for name in f_type.type._member_names_] - member_names.extend([name.upper() for name in f_type.type._member_names_]) - res = {'enum': member_names} - # JSON - elif f_type.type == type_helpers._JSONDict: - res = { - 'type': 'object', - } - # Hparam class - elif callable(f_type.type): - hparam_class = ensure_hparams_cls(f_type.type) - res = hparam_class.get_json_schema() - else: - raise ValueError('Unexpected type when constructing JSON Schema.') - - return _check_for_list_and_optional(f_type, res) - - -def _check_for_list_and_optional(f_type: type_helpers.HparamsType, schema: Dict[str, Any]) -> Dict[str, Any]: - """Wrap JSON Schema with list schema or optional schema if specified. - """ - res = schema - # Wrap type in list - if f_type.is_list: - res = { - 'type': 'array', - 'items': res, - } - # Wrap type for optional - if f_type.is_optional: - res = { - 'oneOf': [ - { - 'type': 'null' - }, - res, - ] - } - return res