Repository: PennyLaneAI/pennylane
Pull request #1547 (2021-08-17 19:34:21)

[CODE TOGETHER] Validation check for arguments passed to qml.operation.Observable
------------------------------------------------------------------------------------------------------------

**Context:**
Code together! Issue #1507
This is my first pull request, hope it works. 

**Description of the Change:**

Catch IndexError and print more informative error message. 
Also added a test.

**Benefits:**

Compared to the other pull request for the same issue, mine prints an informative error (and the full stack trace) including the name of the observable for _any_ observable, and the test is for a generic observable, not only PauliZ.

**Possible Drawbacks:**


**Related GitHub Issues:**
Issue #1507


ingstra: Oh man, I have no idea why all tests seem to be failing o_o
thisac: Hi @ingstra! Thanks for creating this PR. The issue here seems to be that the `wires` should only be retrieved from the `params` if `wires=None`. By removing the `if wires is None:` line, all operations attempt to use the first element in `params` as `wires` (which will not work).

I hope this helps! Let me know if I can clarifiy this further. Lycka till! 🙂 
codecov[bot]: # [Codecov](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1547?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) Report
> Merging [#1547](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1547?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) (15d3b61) into [master](https://codecov.io/gh/PennyLaneAI/pennylane/commit/2cae58d13df401024e24b10bf72d357559182803?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) (2cae58d) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1547/graphs/tree.svg?width=650&height=150&src=pr&token=9S3YzlUgGk&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI)](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1547?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI)

```diff
@@           Coverage Diff           @@
##           master    #1547   +/-   ##
=======================================
  Coverage   98.79%   98.79%           
=======================================
  Files         187      187           
  Lines       13395    13398    +3     
=======================================
+ Hits        13233    13236    +3     
  Misses        162      162           
```


| [Impacted Files](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1547?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI) | Coverage Δ | |
|---|---|---|
| [pennylane/operation.py](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1547/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI#diff-cGVubnlsYW5lL29wZXJhdGlvbi5weQ==) | `98.32% <100.00%> (+0.01%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1547?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1547?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI). Last update [2cae58d...15d3b61](https://codecov.io/gh/PennyLaneAI/pennylane/pull/1547?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=PennyLaneAI).

ingstra: Thanks for the help @thisac , it seems to work now. I saw instructions to also include a change in the changelog for a PR, but I'm unsure where in the changelog? Top? Bottom? 
antalszava: Hi @ingstra, the changelog entry could go [for example here](https://github.com/PennyLaneAI/pennylane/blob/master/.github/CHANGELOG.md#improvements). It's the Improvements section of the v0.18.0 development release.

It seems that two other things will need adjustments:
* One of the unrelated test files was removed (should not be the case)
* The formatting check is failing. See the section on [Continuous Integration Checks in the Development guide](https://pennylane.readthedocs.io/en/stable/development/guide/pullrequests.html#continuous-integration-checks) for instructions on how to aid that.
ingstra: Aw... It was fine before, but I think it got messed up because I was working in my master branch... Now it looks like the proper files are in my repo, but it still seems the file is missing here?
ingstra: @antalszava I'm hopeful it can work now!
antalszava: Hi @ingstra, left two minor suggestions.
ingstra: Thank you @antalszava, your suggestions are good.

Patch:
diff --git a/.github/CHANGELOG.md b/.github/CHANGELOG.md
index 4151ea39f..87ddf5019 100644
--- a/.github/CHANGELOG.md
+++ b/.github/CHANGELOG.md
@@ -13,9 +13,9 @@
 
   - `qml.gradients.vjp`
   - `qml.gradients.batch_vjp`
-  
-* The Hamiltonian can now store grouping information, which can be accessed by a device to 
-  speed up computations of the expectation value of a Hamiltonian. 
+
+* The Hamiltonian can now store grouping information, which can be accessed by a device to
+  speed up computations of the expectation value of a Hamiltonian.
   [(#1515)](https://github.com/PennyLaneAI/pennylane/pull/1515)
 
   ```python
@@ -23,10 +23,10 @@
   coeffs = np.array([1., 2., 3.])
   H = qml.Hamiltonian(coeffs, obs, grouping_type='qwc')
   ```
-  
-  Initialization with a ``grouping_type`` other than ``None`` stores the indices 
-  required to make groups of commuting observables and their coefficients. 
-  
+
+  Initialization with a ``grouping_type`` other than ``None`` stores the indices
+  required to make groups of commuting observables and their coefficients.
+
   ``` pycon
   >>> H.grouping_indices
   [[0, 1], [2]]
@@ -37,7 +37,7 @@
 
   ``` python
   from pennylane import numpy as np
-  
+
   dev = qml.device("default.qubit", wires=2)
   @qml.qnode(dev)
   def circuit(coeffs, param):
@@ -46,7 +46,7 @@
       return qml.expval(
           qml.Hamiltonian(coeffs, [qml.PauliX(0), qml.PauliZ(0)], simplify=True)
       )
-    
+
   coeffs = np.array([-0.05, 0.17])
   param = np.array(1.7)
   grad_fn = qml.grad(circuit)
@@ -84,18 +84,21 @@
 
 <h3>Improvements</h3>
 
+* An error is raised when no arguments are passed to a `qml.operation.Observable` to inform the user about specifying wires.
+  [(#1547)](https://github.com/PennyLaneAI/pennylane/pull/1547)
+
 * The Hamiltonian class was moved to the `ops/qubit` folder from the `vqe` module, since it is now an observable.
   [(#1534)](https://github.com/PennyLaneAI/pennylane/pull/1534)
 
 * The `group_observables` transform is now differentiable.
   [(#1483)](https://github.com/PennyLaneAI/pennylane/pull/1483)
- 
+
   For example:
 
   ``` python
   import jax
   from jax import numpy as jnp
-  
+
   coeffs = jnp.array([1., 2., 3.])
   obs = [PauliX(wires=0), PauliX(wires=1), PauliZ(wires=1)]
 
@@ -111,7 +114,7 @@
   >>> jac_fn(coeffs, select=0)
   [[1. 0. 0.]
   [0. 1. 0.]]
-  
+
   >>> jac_fn(coeffs, select=1)
   [[0., 0., 1.]]
   ```
@@ -142,7 +145,7 @@ and requirements-ci.txt (unpinned). This latter would be used by the CI.
 
 This release contains contributions from (in alphabetical order):
 
-Josh Izaac, Prateek Jain, Johannes Jakob Meyer, Maria Schuld.
+Josh Izaac, Prateek Jain, Johannes Jakob Meyer, Maria Schuld, Ingrid Strandberg.
 
 # Release 0.17.0 (current release)
 
@@ -568,9 +571,9 @@ Josh Izaac, Prateek Jain, Johannes Jakob Meyer, Maria Schuld.
 * The tape does not verify any more that all Observables have owners in the annotated queue.
   [(#1505)](https://github.com/PennyLaneAI/pennylane/pull/1505)
 
-  This allows manipulation of Observables inside a tape context. An example is 
-  `expval(Tensor(qml.PauliX(0), qml.Identity(1)).prune())` which makes the expval an owner 
-  of the pruned tensor and its constituent observables, but leaves the original tensor in 
+  This allows manipulation of Observables inside a tape context. An example is
+  `expval(Tensor(qml.PauliX(0), qml.Identity(1)).prune())` which makes the expval an owner
+  of the pruned tensor and its constituent observables, but leaves the original tensor in
   the queue without an owner.
 
 * The `step` and `step_and_cost` methods of `QNGOptimizer` now accept a custom `grad_fn`
diff --git a/pennylane/operation.py b/pennylane/operation.py
index ea842caa0..8b3509845 100644
--- a/pennylane/operation.py
+++ b/pennylane/operation.py
@@ -1039,8 +1039,14 @@ class Observable(Operator):
     def __init__(self, *params, wires=None, do_queue=True, id=None):
         # extract the arguments
         if wires is None:
-            wires = params[-1]
-            params = params[:-1]
+            try:
+                wires = params[-1]
+                params = params[:-1]
+                # error if no arguments are given
+            except IndexError as err:
+                raise ValueError(
+                    f"Must specify the wires that {type(self).__name__} acts on"
+                ) from err
 
         super().__init__(*params, wires=wires, do_queue=do_queue, id=id)
 


Test patch:
diff --git a/tests/test_operation.py b/tests/test_operation.py
index 9ec1dd4ad..4cf080e26 100644
--- a/tests/test_operation.py
+++ b/tests/test_operation.py
@@ -493,6 +493,19 @@ class TestObservableConstruction:
         assert op.id == "test"
 
 
+class TestObservableInstatiation:
+    """Test that wires are specified when a qml.operation.Observable is instantiated"""
+
+    def test_wire_is_given_in_argument(self):
+        class DummyObservable(qml.operation.Observable):
+            num_wires = 1
+            num_params = 0
+            par_domain = None
+
+        with pytest.raises(Exception, match="Must specify the wires *"):
+            DummyObservable()
+
+
 class TestOperatorIntegration:
     """Integration tests for the Operator class"""