Repository: Quantum-Accelerators/quacc (python)

Issue:
Using environment variables, there is no easy way to set a setting as `None`
### What would you like to report?

I use parsl but I often want to do quick tests without workflow engines.

For this I would like to be able to do something like export QUACC_WORKFLOW_ENGINE = "local" or QUACC_WORKFLOW_ENGINE =

But this gets shutdown by an error. "  Input should be 'covalent', 'dask', 'parsl', 'prefect', 'redun' or 'jobflow' [type=literal_error, input_value='', input_type=str]"

Currently I have no way to run them locally, since I have parsl installed, quacc will by default set the workflow engine setting to parsl.

Pull request:
More robust handling of complex settings types
Closes #1688. Providing a string of "none" or "null" (case-insensitive) will automatically transform to `None`, whether via the CLI, YAML, or environment variables. Same for "true" or "false" (case-insensitive) to `True` or `False`. This is all done at model validation time so we handle all the various settings management cases in one spot.

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 89608d76..e18eff83 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -4,6 +4,13 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [0.6.6]
+
+### Fixed
+
+- Using environment variables to set a setting to `None` now works as expected
+- Overall, more intuitive and robust handling of settings across the various input methods
+
 ## [0.6.5]
 
 ### Added
diff --git a/docs/user/settings/settings.md b/docs/user/settings/settings.md
index bd6ca45b..b82b4067 100644
--- a/docs/user/settings/settings.md
+++ b/docs/user/settings/settings.md
@@ -32,7 +32,13 @@ WORKFLOW_ENGINE: # (3)!
 
 ## Using Environment Variables
 
-If you want to define quacc settings without writing them to a YAML file, you can instead modify the desired settings by defining individual environment variables with `QUACC` as the prefix. For instance, to modify the `SCRATCH_DIR` setting to be `$SCRATCH`, simply define `QUACC_SCRATCH_DIR=$SCRATCH` as a new environment variable. The environment variable takes precedence over any value specified in the YAML file. Most simple field types (e.g. `int`, `float`, `str`) will be automatically inferred from the environment variable.
+If you want to define quacc settings without writing them to a YAML file, you can instead modify the desired settings by defining individual environment variables with `QUACC` as the prefix. The environment variable takes precedence over any value specified in the YAML file. Most simple field types (e.g. `int`, `bool`, `float`, `str`) will be automatically inferred from the environment variable. To achieve the same results as the aforementioned YAML file, you would define the following environment variables:
+
+```bash
+export QUACC_SCRATCH_DIR=$SCRATCH
+export QUACC_CREATE_UNIQUE_DIR=False
+export QUACC_WORKFLOW_ENGINE=None
+```
 
 For more complex types, such as lists or dictionaries, refer to the corresponding section in the [pydantic-settings](https://docs.pydantic.dev/latest/concepts/pydantic_settings/#parsing-environment-variable-values) documentation. We use `__` as the delimiter for nested settings.
 
diff --git a/src/quacc/_cli/quacc.py b/src/quacc/_cli/quacc.py
index c5e870da..fbd11458 100644
--- a/src/quacc/_cli/quacc.py
+++ b/src/quacc/_cli/quacc.py
@@ -83,7 +83,6 @@ def set_(parameter: str, new_value: str) -> None:
     parameter = parameter.upper()
 
     _parameter_handler(parameter, SETTINGS.model_dump())
-    new_value = _type_handler(new_value)
 
     rich_print(f"Setting `{parameter}` to `{new_value}` in {CONFIG_FILE}")
     _update_setting(parameter, new_value, CONFIG_FILE)
@@ -166,26 +165,6 @@ def _parameter_handler(parameter: str, settings_dict: dict) -> None:
         raise ValueError(msg)
 
 
-def _type_handler(value: str) -> Any:
-    """
-    Convert the string value to the appropriate type.
-
-    Parameters
-    ----------
-    value
-        The value to convert.
-
-    Returns
-    -------
-    Any
-    """
-    if value.lower() in {"null", "none"}:
-        value = None
-    elif value.lower() in ("true", "false"):
-        value = value.lower() == "true"
-    return value
-
-
 def _delete_setting(key: str, config_file: Path) -> None:
     """
     Remove the quacc setting from the configuration file.
diff --git a/src/quacc/settings.py b/src/quacc/settings.py
index d214bd72..dd3cfa1d 100644
--- a/src/quacc/settings.py
+++ b/src/quacc/settings.py
@@ -450,18 +450,23 @@ class QuaccSettings(BaseSettings):
         else:
             return v
 
-    model_config = SettingsConfigDict(env_prefix="quacc_", env_nested_delimiter="__")
+    model_config = SettingsConfigDict(
+        env_prefix="quacc_",
+        env_nested_delimiter="__",
+        extra="forbid",
+        validate_assignment=True,
+    )
 
     @model_validator(mode="before")
     @classmethod
-    def load_default_settings(cls, values: dict[str, Any]) -> dict[str, Any]:
+    def load_user_settings(cls, settings: dict[str, Any]) -> dict[str, Any]:
         """
         Loads settings from a root file if available and uses that as defaults in place
-        of built in defaults.
+        of built in defaults. Will also convert common strings to their proper types.
 
         Parameters
         ----------
-        values
+        settings
             Settings to load.
 
         Returns
@@ -469,18 +474,57 @@ class QuaccSettings(BaseSettings):
         dict
             Loaded settings.
         """
+        return _type_handler(_use_custom_config_settings(settings))
+
+
+def _use_custom_config_settings(settings: dict[str, Any]) -> dict[str, Any]:
+    """Parse user settings from a custom YAML.
+
+    Parameters
+    ----------
+    settings : dict
+        Initial settings.
+
+    Returns
+    -------
+    dict
+        Updated settings based on the custom YAML.
+    """
+    from monty.serialization import loadfn
 
-        from monty.serialization import loadfn
+    config_file_path = (
+        Path(settings.get("CONFIG_FILE", _DEFAULT_CONFIG_FILE_PATH))
+        .expanduser()
+        .resolve()
+    )
+
+    new_settings = {}  # type: dict
+    if config_file_path.exists() and config_file_path.stat().st_size > 0:
+        new_settings |= loadfn(config_file_path)
+
+    new_settings.update(settings)
+    return new_settings
 
-        config_file_path = (
-            Path(values.get("CONFIG_FILE", _DEFAULT_CONFIG_FILE_PATH))
-            .expanduser()
-            .resolve()
-        )
 
-        new_values = {}  # type: dict
-        if config_file_path.exists() and config_file_path.stat().st_size > 0:
-            new_values |= loadfn(config_file_path)
+def _type_handler(settings: dict[str, Any]) -> dict[str, Any]:
+    """
+    Convert common strings to their proper types.
+
+    Parameters
+    ----------
+    settings : dict
+        Initial settings.
 
-        new_values.update(values)
-        return new_values
+    Returns
+    -------
+    dict
+        Updated settings.
+    """
+    for key, value in settings.items():
+        if isinstance(value, str):
+            if value.lower() in {"null", "none"}:
+                settings[key] = None
+            elif value.lower() in {"true", "false"}:
+                settings[key] = value.lower() == "true"
+
+    return settings


Test patch:
diff --git a/tests/core/calculators/vasp/test_vasp.py b/tests/core/calculators/vasp/test_vasp.py
index 2d24bac2..005cc79f 100644
--- a/tests/core/calculators/vasp/test_vasp.py
+++ b/tests/core/calculators/vasp/test_vasp.py
@@ -794,13 +794,13 @@ def test_constraints():
 
 def test_envvars():
     DEFAULT_SETTINGS = SETTINGS.model_copy()
-    SETTINGS.VASP_PP_PATH = "/path/to/pseudos"
-    SETTINGS.VASP_VDW = "/path/to/kernel"
+    SETTINGS.VASP_PP_PATH = str(Path("/path/to/pseudos"))
+    SETTINGS.VASP_VDW = str(Path("/path/to/kernel"))
 
     atoms = bulk("Cu")
     atoms.calc = Vasp(atoms, xc="beef-vdw")
-    assert os.environ.get("VASP_PP_PATH") == "/path/to/pseudos"
-    assert os.environ.get("ASE_VASP_VDW") == "/path/to/kernel"
+    assert os.environ.get("VASP_PP_PATH") == str(Path("/path/to/pseudos"))
+    assert os.environ.get("ASE_VASP_VDW") == str(Path("/path/to/kernel"))
 
     SETTINGS.VASP_PP_PATH = DEFAULT_SETTINGS.VASP_PP_PATH
     SETTINGS.VASP_VDW = DEFAULT_SETTINGS.VASP_VDW
diff --git a/tests/core/cli/test_cli.py b/tests/core/cli/test_cli.py
index afdd1520..14062402 100644
--- a/tests/core/cli/test_cli.py
+++ b/tests/core/cli/test_cli.py
@@ -40,16 +40,6 @@ def test_help(runner):
 
 
 def test_set(runner):
-    response = runner.invoke(app, ["set", "WORKFLOW_ENGINE", "None"])
-    assert response.exit_code == 0
-    assert "None" in response.stdout
-    val = None
-    with open(TEST_YAML) as f:
-        for line in f:
-            if "WORKFLOW_ENGINE" in line:
-                val = line.split(":")[-1].strip()
-    assert not val
-
     response = runner.invoke(app, ["set", "WORKFLOW_ENGINE", "covalent"])
     assert response.exit_code == 0
     assert "covalent" in response.stdout
@@ -59,7 +49,6 @@ def test_set(runner):
             if "WORKFLOW_ENGINE" in line:
                 val = line.split(":")[-1].strip()
     assert val == "covalent"
-
     response = runner.invoke(app, ["set", "VASP_PARALLEL_CMD", "dummy"])
     assert response.exit_code == 0
     assert "dummy" in response.stdout
@@ -70,16 +59,6 @@ def test_set(runner):
                 val = line.split(":")[-1].strip()
     assert val == "dummy"
 
-    response = runner.invoke(app, ["set", "GZIP_FILES", "True"])
-    assert response.exit_code == 0
-    assert "True" in response.stdout
-    val = None
-    with open(TEST_YAML) as f:
-        for line in f:
-            if "GZIP_FILES" in line:
-                val = line.split(":")[-1].strip()
-    assert val == "true"
-
 
 def test_unset(runner):
     response = runner.invoke(app, ["unset", "WORKFLOW_ENGINE"])
diff --git a/tests/core/recipes/gulp_recipes/test_gulp_recipes.py b/tests/core/recipes/gulp_recipes/test_gulp_recipes.py
index 350869d9..2ebd18cc 100644
--- a/tests/core/recipes/gulp_recipes/test_gulp_recipes.py
+++ b/tests/core/recipes/gulp_recipes/test_gulp_recipes.py
@@ -1,4 +1,5 @@
 import os
+from pathlib import Path
 
 from ase.build import bulk, molecule
 
@@ -141,7 +142,7 @@ def test_envvars(tmp_path, monkeypatch):
 
     atoms = molecule("H2O")
 
-    SETTINGS.GULP_LIB = "/path/to/lib"
+    SETTINGS.GULP_LIB = str(Path("/path/to/lib"))
     assert static_job(atoms)
-    assert os.environ.get("GULP_LIB") == "/path/to/lib"
+    assert os.environ.get("GULP_LIB") == str(Path("/path/to/lib"))
     SETTINGS.GULP_LIB = DEFAULT_SETTINGS.GULP_LIB
diff --git a/tests/core/settings/test_settings.py b/tests/core/settings/test_settings.py
index db233e22..a2f5e3fd 100644
--- a/tests/core/settings/test_settings.py
+++ b/tests/core/settings/test_settings.py
@@ -8,37 +8,19 @@ from quacc import SETTINGS
 from quacc.recipes.emt.core import relax_job, static_job
 from quacc.settings import QuaccSettings
 
-DEFAULT_SETTINGS = SETTINGS.model_copy()
-
-
 FILE_DIR = Path(__file__).parent
 
 
-def setup_function():
-    SETTINGS.STORE = None
-    SETTINGS.GZIP_FILES = True
-    SETTINGS.CREATE_UNIQUE_DIR = False
-
-
-def teardown_function():
-    SETTINGS.STORE = DEFAULT_SETTINGS.STORE
-    SETTINGS.GZIP_FILES = DEFAULT_SETTINGS.GZIP_FILES
-    SETTINGS.CREATE_UNIQUE_DIR = DEFAULT_SETTINGS.CREATE_UNIQUE_DIR
-
-
-def test_file_v1(tmp_path, monkeypatch):
-    monkeypatch.chdir(tmp_path)
-
-    with open("quacc_test.yaml", "w") as f:
-        f.write("GZIP_FILES: false\nWORKFLOW_ENGINE:\nDEBUG: true")
-    monkeypatch.setenv(
-        "QUACC_CONFIG_FILE", os.path.join(os.getcwd(), "quacc_test.yaml")
-    )
+def test_file(tmp_path, monkeypatch):
+    with open(tmp_path / "quacc_test.yaml", "w") as f:
+        f.write("GZIP_FILES: false\nWORKFLOW_ENGINE: None\nDEBUG: True\nSTORE: null")
+    monkeypatch.setenv("QUACC_CONFIG_FILE", os.path.join(tmp_path, "quacc_test.yaml"))
 
     assert QuaccSettings().GZIP_FILES is False
     assert QuaccSettings().WORKFLOW_ENGINE is None
     assert QuaccSettings().DEBUG is True
-    os.remove("quacc_test.yaml")
+    assert QuaccSettings().STORE is None
+    os.remove(tmp_path / "quacc_test.yaml")
 
 
 def test_store(tmp_path, monkeypatch):
@@ -64,15 +46,35 @@ def test_results_dir(tmp_path, monkeypatch):
 def test_env_var(monkeypatch, tmp_path):
     p = tmp_path / "my/scratch/dir"
     monkeypatch.setenv("QUACC_SCRATCH_DIR", p)
-    assert p.expanduser().resolve() == QuaccSettings().SCRATCH_DIR
+    assert QuaccSettings().SCRATCH_DIR == p.expanduser().resolve()
 
 
-def test_yaml(tmp_path, monkeypatch):
-    monkeypatch.chdir(tmp_path)
+def test_env_var2(monkeypatch, tmp_path):
+    with open(tmp_path / "quacc_test.yaml", "w") as f:
+        f.write("")
 
+    monkeypatch.setenv("QUACC_CONFIG_FILE", os.path.join(tmp_path, "quacc_test.yaml"))
+    assert str(QuaccSettings().CONFIG_FILE) == str(
+        Path(os.path.join(tmp_path, "quacc_test.yaml"))
+    )
+
+    monkeypatch.setenv("QUACC_WORKFLOW_ENGINE", "None")
+    assert QuaccSettings().WORKFLOW_ENGINE is None
+
+    monkeypatch.setenv("QUACC_WORKFLOW_ENGINE", "null")
+    assert QuaccSettings().WORKFLOW_ENGINE is None
+
+    monkeypatch.setenv("QUACC_WORKFLOW_ENGINE", "none")
+    assert QuaccSettings().WORKFLOW_ENGINE is None
+
+    monkeypatch.setenv("QUACC_GZIP_FILES", "FaLsE")
+    assert QuaccSettings().GZIP_FILES is False
+
+
+def test_yaml(tmp_path, monkeypatch):
     p = tmp_path / "my/new/scratch/dir"
     monkeypatch.delenv("QUACC_SCRATCH_DIR", raising=False)
-    with open("quacc_test.yaml", "w") as f:
+    with open(tmp_path / "quacc_test.yaml", "w") as f:
         f.write(f"SCRATCH_DIR: {p}")
-    monkeypatch.setenv("QUACC_CONFIG_FILE", "quacc_test.yaml")
-    assert p.expanduser().resolve() == QuaccSettings().SCRATCH_DIR
+    monkeypatch.setenv("QUACC_CONFIG_FILE", os.path.join(tmp_path, "quacc_test.yaml"))
+    assert QuaccSettings().SCRATCH_DIR == p.expanduser().resolve()