Repository: pybamm-team/PyBaMM
Pull request #4517 (2024-10-15 13:00:00)

Add _from_json functionality for pybamm.sign
# Description

`_from_json` in `pybamm.Sign` was previously labelled as NotImplemented; #4394 identified this as a mistake as the `pybamm.Sign` class should be able to be de/serialised. This PR adds that functionality in.

## Type of change

Please add a line in the relevant section of [CHANGELOG.md](https://github.com/pybamm-team/PyBaMM/blob/develop/CHANGELOG.md) to document the change (include PR #) - note reverse order of PR #s. If necessary, also add to the list of breaking changes.

- [x] Bug fix (non-breaking change which fixes an issue)

# Key checklist:

- [x] No style issues: `$ pre-commit run` (or `$ nox -s pre-commit`) (see [CONTRIBUTING.md](https://github.com/pybamm-team/PyBaMM/blob/develop/CONTRIBUTING.md#installing-and-using-pre-commit) for how to set this up to run automatically when committing locally, in just two lines of code)
- [ ] All tests pass: `$ python run-tests.py --all` (or `$ nox -s tests`)
- [x] The documentation builds: `$ python run-tests.py --doctest` (or `$ nox -s doctests`)

You can run integration tests, unit tests, and doctests together at once, using `$ python run-tests.py --quick` (or `$ nox -s quick`).

## Further checks:

- [ ] Code is commented, particularly in hard-to-understand areas
- [x] Tests added that prove fix is effective or that feature works



Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 762a23d01..12a7d921d 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -21,6 +21,7 @@
 
 ## Bug Fixes
 
+- Added `_from_json()` functionality to `Sign` which was erroneously omitted previously. ([#4517](https://github.com/pybamm-team/PyBaMM/pull/4517))
 - Fixed bug where IDAKLU solver failed when `output variables` were specified and an extrapolation event is present. ([#4440](https://github.com/pybamm-team/PyBaMM/pull/4440))
 
 ## Breaking changes
diff --git a/src/pybamm/expression_tree/unary_operators.py b/src/pybamm/expression_tree/unary_operators.py
index ace1cd994..aa90fd6f4 100644
--- a/src/pybamm/expression_tree/unary_operators.py
+++ b/src/pybamm/expression_tree/unary_operators.py
@@ -212,7 +212,8 @@ class Sign(UnaryOperator):
 
     @classmethod
     def _from_json(cls, snippet: dict):
-        raise NotImplementedError()
+        """See :meth:`pybamm.UnaryOperator._from_json()`."""
+        return cls(snippet["children"][0])
 
     def diff(self, variable):
         """See :meth:`pybamm.Symbol.diff()`."""


Test patch:
diff --git a/tests/unit/test_expression_tree/test_unary_operators.py b/tests/unit/test_expression_tree/test_unary_operators.py
index 0dbafa38c..d7544763f 100644
--- a/tests/unit/test_expression_tree/test_unary_operators.py
+++ b/tests/unit/test_expression_tree/test_unary_operators.py
@@ -138,9 +138,20 @@ class TestUnaryOperators:
         )
 
         # Test from_json
-        with pytest.raises(NotImplementedError):
-            # signs are always scalar/array types in a discretised model
-            pybamm.Sign._from_json({})
+        c = pybamm.Multiplication(pybamm.Variable("a"), pybamm.Scalar(3))
+        sign_json = {
+            "name": "sign",
+            "id": 5341515228900508018,
+            "domains": {
+                "primary": [],
+                "secondary": [],
+                "tertiary": [],
+                "quaternary": [],
+            },
+            "children": [c],
+        }
+
+        assert pybamm.sign(c) == pybamm.Sign._from_json(sign_json)
 
     def test_floor(self, mocker):
         a = pybamm.Symbol("a")