Repository: XanaduAI/flamingpy (language: python)

Issue:
Rethinking the `dims` parameter in `draw_EGraph`
## Context for changes

Introduced a more intuitive `dimensions` parameter that could replace the `dims` attribute of the `EGraph` class.

- **Improvements**: 
    * Adjusted the use of `dims` in the `draw_EGraph` method. Introduced a more intuitive `dimensions` parameter that sets the limits of the x, y, and z axes. Now, `dimensions` is a keyword argument of the `draw_EGraph` method, as opposed to being an attribute of the `EGraph` object like `dims`. If `dimensions` is not passed, the axis limits are calculated (smallest rectangle encapsulating the whole graph). Resolves issue #40.
- **Documentation changes**:
    * Added documentation on how the new `dimensions` parameter should be used.

## Example usage and tests

![image](https://user-images.githubusercontent.com/44366402/168667717-c56fd155-3f20-488c-a295-624097e9f0cc.png)

![image](https://user-images.githubusercontent.com/44366402/168667806-57ae59b5-ed01-4510-96ad-0387f62e5349.png)

![image](https://user-images.githubusercontent.com/44366402/168667989-5ac9daea-0de3-4332-873f-84bfb6c70dc0.png)


## Expected benefits and drawbacks

**Expected benefits:**
-  Easier and more intuitive way to draw `EGraph` objects. This version no longer requires the `EGraph` to have a `dims` attribute when drawing it.

**Possible drawbacks:**
- The `dims` parameter might be used in other places that I am not aware of. The new `dimensions` parameter might be confusing if this is the case.
- For very large graphs the calculation of the `mins = tuple(map(min, zip(*egraph.nodes)))` and `maxs = tuple(map(max, zip(*egraph.nodes)))` might not be desired. However, I don't think that should be a bottleneck when drawing large graphs.

## Related Github issues
Resolves #40 

## Checklist and integration statements

- [x] My Python and C++ codes follow the coding and commenting styles of this project as indicated by existing files. Specifically, the changes conform to given `black`, `docformatter` and `pylint` configurations. 
- [x] I have performed a self-review of these changes, and checked my code (including for [codefactor](https://www.codefactor.io/repository/github/xanaduai/flamingpy/branches) compliance), and corrected misspellings to the best of my capacity. I have synced this branch with others as required.
- [x] I have added context for corresponding changes in documentation and [`README.md`](README.md) as needed.
- [ ] I have added new workflow CI tests for corresponding changes, ensuring codecoverage 95% or better, and these pass locally for me.
- [x] I have updated [`CHANGELOG.md`](.github/CHANGELOG.md) following the template. I recognize that the developers may revisit [`CHANGELOG.md`](.github/CHANGELOG.md) and the versioning, and create a Special Release including my changes.



Discussion / hints:
For reference, these are the plots/outputs with the current main branch:
![image](https://user-images.githubusercontent.com/44366402/168671064-f10c22e5-ba9a-4d14-a91f-504f8148fe32.png)
![image](https://user-images.githubusercontent.com/44366402/168671185-b0174ce3-f8da-45dc-8a4a-f203af108bc8.png)
![image](https://user-images.githubusercontent.com/44366402/168671329-dd847f8a-1cb0-4662-b857-fdaedf09d0d5.png)

Another thing I'm curious about is why the z- and y-axis are flipped in the current implementation. I guess this was done after some discussion but it does not make immediate sense to (a novel user like) me 🤔 
TODO: Just found out that the current version is not able to show graphs partly. That is, it does not allow for slices to be displayed as you would expect. Ideally, the final version would be something analogous to matplotlib's xlim/ylim where points outside the range are ignored. 
![image](https://user-images.githubusercontent.com/44366402/168674165-459cc08a-598a-4c6e-9549-9516644b308b.png)

Opened PR for review from @ilan-tz. Note, there is one thing I’m not satisfied with - the case where the axis limits don’t contain the full graph. In that case, matplotlib creates an odd image that contains edges and nodes outside the range. We could discuss what the desired solution is in that case.
# [Codecov](https://codecov.io/gh/XanaduAI/flamingpy/pull/42?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI) Report
> Merging [#42](https://codecov.io/gh/XanaduAI/flamingpy/pull/42?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI) (34e4176) into [main](https://codecov.io/gh/XanaduAI/flamingpy/commit/9cbc931038be123ea986b21f979e9a79f869a18e?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI) (9cbc931) will **decrease** coverage by `0.07%`.
> The diff coverage is `88.88%`.

```diff
@@            Coverage Diff             @@
##             main      #42      +/-   ##
==========================================
- Coverage   96.13%   96.05%   -0.08%     
==========================================
  Files          35       35              
  Lines        2145     2154       +9     
==========================================
+ Hits         2062     2069       +7     
- Misses         83       85       +2     
```

| [Impacted Files](https://codecov.io/gh/XanaduAI/flamingpy/pull/42?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI) | Coverage Δ | |
|---|---|---|
| [flamingpy/utils/viz.py](https://codecov.io/gh/XanaduAI/flamingpy/pull/42/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI#diff-ZmxhbWluZ3B5L3V0aWxzL3Zpei5weQ==) | `89.59% <86.66%> (-0.29%)` | :arrow_down: |
| [flamingpy/\_version.py](https://codecov.io/gh/XanaduAI/flamingpy/pull/42/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI#diff-ZmxhbWluZ3B5L192ZXJzaW9uLnB5) | `100.00% <100.00%> (ø)` | |
| [flamingpy/codes/surface\_code.py](https://codecov.io/gh/XanaduAI/flamingpy/pull/42/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI#diff-ZmxhbWluZ3B5L2NvZGVzL3N1cmZhY2VfY29kZS5weQ==) | `97.31% <100.00%> (ø)` | |
| [flamingpy/examples/graphstates.py](https://codecov.io/gh/XanaduAI/flamingpy/pull/42/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI#diff-ZmxhbWluZ3B5L2V4YW1wbGVzL2dyYXBoc3RhdGVzLnB5) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/XanaduAI/flamingpy/pull/42?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI).
> **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=XanaduAI)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/XanaduAI/flamingpy/pull/42?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI). Last update [9cbc931...34e4176](https://codecov.io/gh/XanaduAI/flamingpy/pull/42?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=XanaduAI). 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=XanaduAI).

@ilan-tz it's ready for review. Note that if the user specifies limits that do not capture the full graph state, the plot will not truncate it appropriately (see below). As discussed, this might not be crucial and could be fixed/adjusted in a future PR.

![image](https://user-images.githubusercontent.com/44366402/170347624-6bf935db-4bb1-4f78-a130-7028ce0f37ad.png)
![image](https://user-images.githubusercontent.com/44366402/170347237-f8243f56-d8b0-4f12-bd0e-cdda4d7d11f9.png)
![image](https://user-images.githubusercontent.com/44366402/170347528-f507f1c3-5059-4853-8710-f68b302a180c.png)
Related SE post: https://stackoverflow.com/questions/44091229/trim-data-outside-3d-plot-in-matplotlib
Hi @soosub ! I was close to merging this but encountered the following message:

![warning](https://user-images.githubusercontent.com/57886357/170367921-a0675b17-29a0-4982-8c08-7f51422bdcdf.png)

Let's investigate this before merging. 
@ilan-tz I suspect that I know where that comes from. This some GHZ cluster state in 1D probably? If so, I'll fix it before tomorrow 
> @ilan-tz I suspect that I know where that comes from. This some GHZ cluster state in 1D probably? If so, I'll fix it before tomorrow 

Yup, a GHZ state. Comes from running the graph state tutorial.
@ilan-tz that should fix it! I added a check to see if the x, y and z axis limits on either side are not identical
@soosub, you did not tick any items in your checklist in the description above. Any particular reason? Ideally, we want devs to confirm all. 
> @soosub, you did not tick any items in your checklist in the description above. Any particular reason? Ideally, we want devs to confirm all.

Nope, will make sure to do that in the future. 

The only thing I did not add is new tests, I could do that tomorrow if necessary. What do you think @nariman87 ?
> > @soosub, you did not tick any items in your checklist in the description above. Any particular reason? Ideally, we want devs to confirm all.
> 
> Nope, will make sure to do that in the future.
> 
> The only thing I did not add is new tests, I could do that tomorrow if necessary. What do you think @nariman87 ?

Thanks, new tests are very important, please do that with no hurries and then mark ready again.

UPDATE: will do new tests for viz.py in separate PRs.

Fix patch:
diff --git a/.github/CHANGELOG.md b/.github/CHANGELOG.md
index 9aac87fd..50925cee 100644
--- a/.github/CHANGELOG.md
+++ b/.github/CHANGELOG.md
@@ -8,7 +8,8 @@
 
 ### Improvements
 * Pylint is pinned to stable version `pylint==2.14.0` and added to `dev_requirements.txt`. [#76](https://github.com/XanaduAI/flamingpy/pull/76)
- * pylint no-self-use tags are removed as this check has been removed from pylint (see [here](https://github.com/PyCQA/pylint/issues/5502)). 
+  * pylint no-self-use tags are removed as this check has been removed from pylint (see [here](https://github.com/PyCQA/pylint/issues/5502)). 
+* Added tests for `EGraph` plots. [#60](https://github.com/XanaduAI/flamingpy/pull/60)
 
 ### Documentation changes
 
@@ -17,7 +18,7 @@
 
 This release contains contributions from (in alphabetical order):
 
-[Sebastián Duque Mesa](https://github.com/sduquemesa)
+[Joost Bus](https://github.com/soosub), [Sebastián Duque Mesa](https://github.com/sduquemesa)
 
 See full commit details ...
 
diff --git a/flamingpy/_version.py b/flamingpy/_version.py
index b9087ac4..80b1830d 100644
--- a/flamingpy/_version.py
+++ b/flamingpy/_version.py
@@ -15,4 +15,4 @@
 """Version number (major.minor.patch[label])"""
 
 
-__version__ = "0.8.2a5-dev0"
+__version__ = "0.8.2a5-dev1"


Test patch:
diff --git a/tests/unit/test_viz.py b/tests/unit/test_viz.py
index 54a77e3b..32c48d72 100644
--- a/tests/unit/test_viz.py
+++ b/tests/unit/test_viz.py
@@ -14,10 +14,17 @@
 """"Unit tests for functions in the viz module."""
 
 import math
+from datetime import datetime
+
 import numpy as np
 from numpy.random import default_rng as rng
+import pytest
+import matplotlib.pyplot as plt
+
 
-from flamingpy.utils.viz import to_pi_string
+from flamingpy.utils.viz import to_pi_string, draw_EGraph
+from flamingpy.codes.graphs import EGraph
+from flamingpy.codes import SurfaceCode
 
 
 def test_to_pi_string():
@@ -44,3 +51,37 @@ def test_to_pi_string():
 
     # Test for tex=False
     assert to_pi_string(-np.sqrt(np.pi) / 2, tex=False) == "-\\sqrt{\\pi}/2"
+
+
+def test_draw_egraph_bell():
+    """Test for the draw method of EGraph of Bell state."""
+    # Bell state EGraph
+    edge = [(0, 0, 0), (0, 0, 1)]
+    bell_state = EGraph()
+    bell_state.add_edge(*edge, color="MidnightBlue")
+
+    # Test for drawing the EGraph
+    _, a = draw_EGraph(bell_state)
+    plt.close()
+
+    assert len(a.get_xticks()) == 1
+    assert a.get_xlim() == (-1, 1)
+
+
+@pytest.mark.parametrize("d", (2, 3))
+def test_draw_egraph_rhg(d):
+    """Test for the draw method of EGraph of RHG lattice."""
+    # Bell state EGraph
+    RHG = SurfaceCode(d).graph
+
+    # Test for drawing the EGraph
+    _, a = draw_EGraph(RHG)
+    plt.close()
+
+    n_ticks = 2 * d - 1
+
+    ticks = (a.get_xticks(), a.get_yticks(), a.get_zticks())
+    assert [len(tick) for tick in ticks] == [n_ticks] * 3
+
+    actual_lims = (a.get_xlim(), a.get_ylim(), a.get_zlim())
+    assert actual_lims == ((0, n_ticks - 1), (1, n_ticks), (1, n_ticks))