Allow overrides to be JSON string or dict (#4680)

Author: mulhod

CHANGELOG.md

@@ -28,6 +28,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- Allow overrides to be JSON string or `dict`.
 - `transformers` dependency updated to version 3.1.0.
 - When `cached_path` is called on a local archive with `extract_archive=True`, the archive is now extracted into a unique subdirectory of the cache root instead of a subdirectory of the archive's directory. The extraction directory is also unique to the modification time of the archive, so if the file changes, subsequent calls to `cached_path` will know to re-extract the archive.
 - Removed the `truncation_strategy` parameter to `PretrainedTransformerTokenizer`. The way we're calling the tokenizer, the truncation strategy takes no effect anyways.

allennlp/commands/train.py

@@ -122,7 +122,7 @@ def train_model_from_args(args: argparse.Namespace):
 def train_model_from_file(
     parameter_filename: Union[str, PathLike],
     serialization_dir: Union[str, PathLike],
-    overrides: str = "",
+    overrides: Union[str, Dict[str, Any]] = "",
     recover: bool = False,
     force: bool = False,
     node_rank: int = 0,
@@ -140,8 +140,8 @@ def train_model_from_file(
     serialization_dir : `str`
         The directory in which to save results and logs. We just pass this along to
         [`train_model`](#train_model).
-    overrides : `str`
-        A JSON string that we will use to override values in the input parameter file.
+    overrides : `Union[str, Dict[str, Any]]`, optional (default = `""`)
+        A JSON string or a dict that we will use to override values in the input parameter file.
     recover : `bool`, optional (default=`False`)
         If `True`, we will try to recover a training run from an existing serialization
         directory.  This is only intended for use when something actually crashed during the middle

allennlp/common/params.py

@@ -457,7 +457,10 @@ def _check_is_dict(self, new_history, value):
 
     @classmethod
     def from_file(
-        cls, params_file: Union[str, PathLike], params_overrides: str = "", ext_vars: dict = None
+        cls,
+        params_file: Union[str, PathLike],
+        params_overrides: Union[str, Dict[str, Any]] = "",
+        ext_vars: dict = None,
     ) -> "Params":
         """
         Load a `Params` object from a configuration file.
@@ -468,7 +471,7 @@ def from_file(
 
             The path to the configuration file to load.
 
-        params_overrides: `str`, optional
+        params_overrides: `Union[str, Dict[str, Any]]`, optional (default = `""`)
 
             A dict of overrides that can be applied to final object.
             e.g. {"model.embedding_dim": 10}
@@ -490,6 +493,8 @@ def from_file(
 
         file_dict = json.loads(evaluate_file(params_file, ext_vars=ext_vars))
 
+        if isinstance(params_overrides, dict):
+            params_overrides = json.dumps(params_overrides)
         overrides_dict = parse_overrides(params_overrides)
         param_dict = with_fallback(preferred=overrides_dict, fallback=file_dict)
 

allennlp/models/archival.py

@@ -2,7 +2,7 @@
 Helper functions for archiving models and restoring archived models.
 """
 from os import PathLike
-from typing import NamedTuple, Union
+from typing import NamedTuple, Union, Dict, Any
 import logging
 import os
 import tempfile
@@ -132,7 +132,7 @@ def archive_model(
 def load_archive(
     archive_file: Union[str, Path],
     cuda_device: int = -1,
-    overrides: str = "",
+    overrides: Union[str, Dict[str, Any]] = "",
     weights_file: str = None,
 ) -> Archive:
     """
@@ -145,7 +145,7 @@ def load_archive(
     cuda_device : `int`, optional (default = `-1`)
         If `cuda_device` is >= 0, the model will be loaded onto the
         corresponding GPU. Otherwise it will be loaded onto the CPU.
-    overrides : `str`, optional (default = `""`)
+    overrides : `Union[str, Dict[str, Any]]`, optional (default = `""`)
         JSON overrides to apply to the unarchived `Params` object.
     weights_file : `str`, optional (default = `None`)
         The weights file to use.  If unspecified, weights.th in the archive_file will be used.

allennlp/predictors/predictor.py

@@ -239,7 +239,7 @@ def from_path(
         dataset_reader_to_load: str = "validation",
         frozen: bool = True,
         import_plugins: bool = True,
-        overrides: str = "",
+        overrides: Union[str, Dict[str, Any]] = "",
     ) -> "Predictor":
         """
         Instantiate a `Predictor` from an archive path.
@@ -267,7 +267,7 @@ def from_path(
             This comes with additional overhead, but means you don't need to explicitly
             import the modules that your predictor depends on as long as those modules
             can be found by `allennlp.common.plugins.import_plugins()`.
-        overrides : `str`, optional (default = `""`)
+        overrides : `Union[str, Dict[str, Any]]`, optional (default = `""`)
             JSON overrides to apply to the unarchived `Params` object.
 
         # Returns

tests/common/params_test.py

@@ -33,14 +33,18 @@ def test_bad_unicode_environment_variables(self):
         Params.from_file(filename)
         del os.environ["BAD_ENVIRONMENT_VARIABLE"]
 
-    def test_overrides(self):
+    @pytest.mark.parametrize("input_type", [dict, str])
+    def test_overrides(self, input_type):
         filename = self.FIXTURES_ROOT / "simple_tagger" / "experiment.json"
-        overrides = (
-            '{ "train_data_path": "FOO", "model": { "type": "BAR" },'
-            '"model.text_field_embedder.tokens.type": "BAZ",'
-            '"data_loader.batch_sampler.sorting_keys.0": "question"}'
+        overrides = {
+            "train_data_path": "FOO",
+            "model": {"type": "BAR"},
+            "model.text_field_embedder.tokens.type": "BAZ",
+            "data_loader.batch_sampler.sorting_keys.0": "question",
+        }
+        params = Params.from_file(
+            filename, overrides if input_type == dict else json.dumps(overrides)
         )
-        params = Params.from_file(filename, overrides)
 
         assert "dataset_reader" in params
         assert "trainer" in params