Repository: mit-ll-responsible-ai/hydra-zen
Pull request #110 (2021-09-29 19:26:35)

Migrate all zen-processing to `_zen_xx`-named fields
## Migrating to `_zen_xx` fields
Per @Jasha10 's recommendation, we want to avoid potential future-conflicts with Hydra keeping away from the `_xx_` naming scheme. Instead we will exclusively reserve fields with the naming scheme `_zen_xx`.

Thus specifying `hydra_partial=True` will now occupy `_zen_target` and `_zen_partial`, instead of `_partial_target_` . Note that this PR preserves compatibility with configs/yamls that *do* specify `_partial_target_`. I.e. this is not a compatibility-breaking change 🥳 .

## Introducing common machinery for all zen-specific processing
This PR also introduces `hydra_zen.funcs.zen_processing`, which will be our "uber" processing function that handles all pre/post processing surrounding instantiation. Here, it simply implements `hydra_partial=True`, but in an upcoming PR, it will also facilitate `hydra_meta`. This will make it much easier to make multiple zen features (e.g. specifying `hydra_partial=True` *and* `hydra_meta=...`) compose, and intermingle.

## Variable-name squatting
Lastly, we will officially have `builds` reserve all names of the form `hydra_xx` so that we never have to worry about "taking" these names from users in future. We also 

## Summary

Before:

```python
>>> from hydra_zen import builds, to_yaml
>>> c = builds(dict, a=1, hydra_partial=True)
>>> print(to_yaml(c))
_target_: hydra_zen.funcs.partial
_partial_target_:
  _target_: hydra_zen.funcs.get_obj
  path: builtins.dict
a: 1
```

After:

```python
>>> print(to_yaml(c))
_target_: hydra_zen.funcs.zen_processing
_zen_target: builtins.dict
_zen_partial: true
a: 1
```

Before:

```python
>>> builds(dict, hydra_something=1)  # OK
>>> builds(dict, _zen_partial=1)  # OK
```


After:

```python
>>> builds(dict, hydra_something=1)  # Error: we reserve all hydra_xx names
>>> builds(dict, _zen_partial=1)  # Error: we reserve all _zen_xx names
```


Patch:
diff --git a/docs/source/changes.rst b/docs/source/changes.rst
index 9ddba1e2..d2ebbdf3 100644
--- a/docs/source/changes.rst
+++ b/docs/source/changes.rst
@@ -8,7 +8,7 @@ All previous releases should still be available on pip.
 .. _v0.3.0:
 
 ---------------------
-0.3.0rc2 - 2021-09-20
+0.3.0rc3 - 2021-09-20
 ---------------------
 
 This release:
@@ -16,6 +16,7 @@ This release:
 - Makes the "target" of `builds` a positional-only argument. Code that specifies ``builds(target=<target>, ...)`` will now raise a deprecation warning. Now, e.g., ``builds(dict, target=1)`` will work. `#104 <https://github.com/mit-ll-responsible-ai/hydra-zen/pull/104>`_
 - Adds the :func:`~hydra_zen.get_target` function for retrieving target-objects from structured configs. `#94 <https://github.com/mit-ll-responsible-ai/hydra-zen/pull/94>`_
 - Improves type-annotations on `builds`. Now, e.g., ``builds("hi")`` will be marked as invalid by static checkers (the target of `builds` must be callable). `#104 <https://github.com/mit-ll-responsible-ai/hydra-zen/pull/104>`_.
+- Migrates zen-specific fields to a new naming-scheme, and zen-specific processing to a universal mechanism. See `#110 <https://github.com/mit-ll-responsible-ai/hydra-zen/pull/110>_` for more details.
 - Ensures that hydra-zen's source code is "pyright-clean", under `pyright's basic type-checking mode <https://github.com/microsoft/pyright/blob/main/docs/configuration.md#diagnostic-rule-defaults>`_. `#101 <https://github.com/mit-ll-responsible-ai/hydra-zen/pull/101>`_
 - Adds to all public modules/packages an ``__all__`` field. `#99 <https://github.com/mit-ll-responsible-ai/hydra-zen/pull/99>`_
 - Adds PEP 561 compliance (e.g. hydra-zen is now compatible with mypy). `#97 <https://github.com/mit-ll-responsible-ai/hydra-zen/pull/97>`_
diff --git a/src/hydra_zen/funcs.py b/src/hydra_zen/funcs.py
index 8f3ee044..f4fab46c 100644
--- a/src/hydra_zen/funcs.py
+++ b/src/hydra_zen/funcs.py
@@ -38,3 +38,10 @@ def get_obj(*, path: str) -> _typing.Union[type, _typing.Callable[..., _typing.A
     except Exception as e:  # pragma: no cover
         _log.error(f"Error getting callable at {path} : {e}")
         raise e
+
+
+def zen_processing(*args, _zen_target: str, _zen_partial: bool = False, **kwargs):
+    obj = get_obj(path=_zen_target)
+    if _zen_partial is True:
+        return _functools.partial(obj, *args, **kwargs)
+    return obj(*args, **kwargs)
diff --git a/src/hydra_zen/structured_configs/_implementations.py b/src/hydra_zen/structured_configs/_implementations.py
index 6640fda7..b7d6f244 100644
--- a/src/hydra_zen/structured_configs/_implementations.py
+++ b/src/hydra_zen/structured_configs/_implementations.py
@@ -4,7 +4,7 @@ import inspect
 import warnings
 from collections import defaultdict
 from dataclasses import Field, dataclass, field, fields, is_dataclass, make_dataclass
-from functools import partial, wraps
+from functools import wraps
 from itertools import chain
 from typing import (
     Any,
@@ -27,7 +27,7 @@ from typing import (
 from typing_extensions import Final, Literal
 
 from hydra_zen.errors import HydraZenDeprecationWarning
-from hydra_zen.funcs import get_obj, partial
+from hydra_zen.funcs import get_obj, partial, zen_processing
 from hydra_zen.structured_configs import _utils
 from hydra_zen.typing import Builds, Importable, Just, PartialBuilds
 from hydra_zen.typing._implementations import HasPartialTarget, HasTarget
@@ -44,7 +44,8 @@ _T = TypeVar("_T")
 _TARGET_FIELD_NAME: Final[str] = "_target_"
 _RECURSIVE_FIELD_NAME: Final[str] = "_recursive_"
 _CONVERT_FIELD_NAME: Final[str] = "_convert_"
-_PARTIAL_TARGET_FIELD_NAME: Final[str] = "_partial_target_"
+_PARTIAL_TARGET_FIELD_NAME: Final[str] = "_zen_partial"
+_ZEN_TARGET_FIELD_NAME: Final[str] = "_zen_target"
 _POS_ARG_FIELD_NAME: Final[str] = "_args_"
 _JUST_FIELD_NAME: Final[str] = "path"
 _HYDRA_FIELD_NAMES: FrozenSet[str] = frozenset(
@@ -52,7 +53,6 @@ _HYDRA_FIELD_NAMES: FrozenSet[str] = frozenset(
         _TARGET_FIELD_NAME,
         _RECURSIVE_FIELD_NAME,
         _CONVERT_FIELD_NAME,
-        _PARTIAL_TARGET_FIELD_NAME,
         _POS_ARG_FIELD_NAME,
     )
 )
@@ -661,6 +661,23 @@ def builds(
             + "`builds(..., hydra_partial=True)` requires that `hydra_recursive=True`"
         )
 
+    # Check for reserved names
+    for name in kwargs_for_target:
+        if name in _HYDRA_FIELD_NAMES:
+            err_msg = f"The field-name specified via `builds(..., {name}=<...>)` is reserved by Hydra."
+            if name != _TARGET_FIELD_NAME:
+                raise ValueError(
+                    err_msg
+                    + f" You can set this parameter via `builds(..., hydra_{name[1:-1]}=<...>)`"
+                )
+            else:
+                raise ValueError(err_msg)
+        if name.startswith(("hydra_", "_zen_")):
+            raise ValueError(
+                f"The field-name specified via `builds(..., {name}=<...>)` is reserved by hydra-zen."
+                " You can manually create a dataclass to utilize this name in a structured config."
+            )
+
     target_field: List[Union[Tuple[str, Type[Any]], Tuple[str, Type[Any], Field[Any]]]]
 
     if hydra_partial is True:
@@ -668,12 +685,17 @@ def builds(
             (
                 _TARGET_FIELD_NAME,
                 str,
-                _utils.field(default=_utils.get_obj_path(partial), init=False),
+                _utils.field(default=_utils.get_obj_path(zen_processing), init=False),
+            ),
+            (
+                _ZEN_TARGET_FIELD_NAME,
+                str,
+                _utils.field(default=_utils.get_obj_path(target), init=False),
             ),
             (
                 _PARTIAL_TARGET_FIELD_NAME,
-                Any,
-                _utils.field(default=just(target), init=False),
+                bool,
+                _utils.field(default=True, init=False),
             ),
         ]
     else:
@@ -771,7 +793,7 @@ def builds(
         _field.name
         for _base in builds_bases
         for _field in fields(_base)
-        if _field.name not in _HYDRA_FIELD_NAMES
+        if _field.name not in _HYDRA_FIELD_NAMES and not _field.name.startswith("_zen_")
     }
 
     # Validate that user-specified arguments satisfy target's signature.
@@ -1007,11 +1029,13 @@ def is_just(x: Any) -> bool:
     return False
 
 
-def is_partial_builds(x: Any) -> bool:
-    if is_builds(x) and hasattr(x, _PARTIAL_TARGET_FIELD_NAME):
+def _is_old_partial_builds(x: Any) -> bool:  # pragma: no cover
+    # We don't care about coverage here.
+    # This will only be used in `get_target` and we'll be sure to cover that branch
+    if is_builds(x) and hasattr(x, "_partial_target_"):
         attr = _get_target(x)
-        if (attr == _get_target(PartialBuilds) or attr is partial) and is_just(
-            getattr(x, _PARTIAL_TARGET_FIELD_NAME)
+        if (attr == "hydra_zen.funcs.partial" or attr is partial) and is_just(
+            getattr(x, "_partial_target_")
         ):
             return True
         else:
@@ -1020,6 +1044,15 @@ def is_partial_builds(x: Any) -> bool:
     return False
 
 
+def is_partial_builds(x: Any) -> bool:
+    if not is_builds(x) or not hasattr(x, _ZEN_TARGET_FIELD_NAME):
+        return False
+    attr = _get_target(x)
+    if attr != _get_target(PartialBuilds) and attr is not zen_processing:
+        return False
+    return getattr(x, _PARTIAL_TARGET_FIELD_NAME, False) is True
+
+
 @overload
 def get_target(obj: Type[PartialBuilds[_T]]) -> _T:  # pragma: no cover
     ...
@@ -1095,9 +1128,11 @@ def get_target(obj: Union[HasTarget, HasPartialTarget]) -> Any:
     >>> get_target(ModelConfig)
     CustomClassifier
     """
-    if is_partial_builds(obj):
+    if _is_old_partial_builds(obj):
         # obj._partial_target_ is `Just[obj]`
-        return get_target(getattr(obj, _PARTIAL_TARGET_FIELD_NAME))
+        return get_target(getattr(obj, "_partial_target_"))
+    elif is_partial_builds(obj):
+        field_name = _ZEN_TARGET_FIELD_NAME
     elif is_just(obj):
         field_name = _JUST_FIELD_NAME
     elif is_builds(obj):
diff --git a/src/hydra_zen/typing/_implementations.py b/src/hydra_zen/typing/_implementations.py
index 4632462f..2451d071 100644
--- a/src/hydra_zen/typing/_implementations.py
+++ b/src/hydra_zen/typing/_implementations.py
@@ -60,9 +60,10 @@ class Just(Builds, Protocol[_T]):  # pragma: no cover
 
 
 @runtime_checkable
-class PartialBuilds(Builds, Protocol[_T2]):  # pragma: no cover
-    _partial_target_: Just[_T2]
-    _target_: str = "hydra_zen.funcs.partial"
+class PartialBuilds(Builds, Protocol[_T]):  # pragma: no cover
+    _target_: str = "hydra_zen.funcs.zen_processing"
+    _zen_target: str
+    _zen_partial: bool = True
 
 
 @runtime_checkable
@@ -72,4 +73,4 @@ class HasTarget(Protocol):  # pragma: no cover
 
 @runtime_checkable
 class HasPartialTarget(Protocol):  # pragma: no cover
-    _partial_target_: Just
+    _zen_partial: bool = True


Test patch:
diff --git a/tests/test_compatibility/test_old_partial_implementation.py b/tests/test_compatibility/test_old_partial_implementation.py
new file mode 100644
index 00000000..f28fa139
--- /dev/null
+++ b/tests/test_compatibility/test_old_partial_implementation.py
@@ -0,0 +1,40 @@
+# Copyright (c) 2021 Massachusetts Institute of Technology
+# SPDX-License-Identifier: MIT
+
+from dataclasses import dataclass
+from typing import Any
+
+from omegaconf import OmegaConf
+
+from hydra_zen import get_target, instantiate, just, to_yaml
+
+
+def f(*args, **kwargs):
+    return args, kwargs
+
+
+# These tests ensure that the implementation of hydra_partial=True
+# for hydra_zen < 0.3.0 still works
+@dataclass
+class OldPartial:
+    _target_: str = "hydra_zen.funcs.partial"
+    _partial_target_: Any = just(f)
+    _args_: Any = ("pos",)
+    a: int = 1
+
+
+def test_old_partial_instantiates():
+    args, kwargs = instantiate(OldPartial)(b=2)
+    assert args == ("pos",)
+    assert kwargs == {"a": 1, "b": 2}
+
+
+def test_old_partial_from_yaml():
+    yaml = to_yaml(OldPartial)
+    args, kwargs = instantiate(OmegaConf.create(yaml))(b=2)
+    assert args == ("pos",)
+    assert kwargs == {"a": 1, "b": 2}
+
+
+def test_old_partial_get_target():
+    assert get_target(OldPartial) is f
diff --git a/tests/test_protocols.py b/tests/test_protocols.py
index 67ac9521..e375231a 100644
--- a/tests/test_protocols.py
+++ b/tests/test_protocols.py
@@ -8,7 +8,7 @@ from typing import Any
 import pytest
 
 from hydra_zen import builds, instantiate, just
-from hydra_zen.funcs import get_obj, partial as hydra_partial
+from hydra_zen.funcs import get_obj, zen_processing
 from hydra_zen.structured_configs._implementations import (
     is_builds,
     is_just,
@@ -80,26 +80,37 @@ class AJust:
 
 @dataclass
 class APartial:
-    _target_: Any = hydra_partial
-    _partial_target_: Any = just(int)
+    _target_: Any = zen_processing
+    _zen_target: Any = "builtins.int"
+    _zen_partial: bool = True
 
 
 @dataclass
 class NotJust:
-    _target_: Any = "builtins.dict"
+    _target_: Any = "builtins.dict"  # wrong target
     path: Any = "builtins.int"
 
 
 @dataclass
 class NotPartial:
-    _target_: Any = "builtins.dict"
-    _partial_target_: Any = just(int)
+    _target_: Any = "builtins.dict"  # wrong target
+    _zen_target: Any = "builtins.int"
+    _zen_partial: bool = True
 
 
 @dataclass
 class NotPartial2:
-    _target_: Any = hydra_partial
-    _partial_target_: Any = int
+    _target_: Any = "builtins.dict"
+    # MISSING _zen_target
+    _zen_partial: bool = True
+
+
+@dataclass
+class NotPartial3:
+    # partial is False
+    _target_: Any = zen_processing
+    _zen_target: Any = "builtins.int"
+    _zen_partial: bool = False  # <- False!
 
 
 @pytest.mark.parametrize(
@@ -114,6 +125,7 @@ class NotPartial2:
         (NotJust, True, False, False),
         (NotPartial, True, False, False),
         (NotPartial2, True, False, False),
+        (NotPartial3, True, False, False),
     ],
 )
 def test_protocol_checkers(x, yes_builds, yes_just, yes_partial):
diff --git a/tests/test_validation.py b/tests/test_validation.py
index f88bf974..766a313b 100644
--- a/tests/test_validation.py
+++ b/tests/test_validation.py
@@ -12,6 +12,10 @@ from omegaconf import OmegaConf
 
 from hydra_zen import builds, get_target, hydrated_dataclass, instantiate, just, to_yaml
 from hydra_zen.errors import HydraZenDeprecationWarning
+from hydra_zen.structured_configs._implementations import (
+    _HYDRA_FIELD_NAMES,
+    _ZEN_TARGET_FIELD_NAME,
+)
 
 
 def test_builds_no_args_raises():
@@ -313,3 +317,14 @@ def test_just_raises_for_unimportable_target():
 def test_get_target_on_non_builds():
     with pytest.raises(TypeError):
         get_target(1)
+
+
+@pytest.mark.parametrize(
+    "field",
+    list(_HYDRA_FIELD_NAMES)
+    + [_ZEN_TARGET_FIELD_NAME, "_zen_some_new_feature", "hydra_some_new_feature"],
+)
+def test_reserved_names_are_reserved(field: str):
+    kwargs = {field: True}
+    with pytest.raises(ValueError):
+        builds(dict, **kwargs)