Repository: scikit-hep/pylhe

Issue:
Reconsider tex2pix dependency
`tex2pix` is a [core dependency of `pylhe`](https://github.com/scikit-hep/pylhe/blob/d9ce20436161ec174bde6555c7dd6e68f7f5e0ea/setup.cfg#L40-L43) but the [last release of `v0.3.1` was in 2016](https://pypi.org/project/tex2pix/0.3.1/#history), so it seems to be safely "unmaintained" at this point. The source code also doesn't seem to be publicly available on GitHub.

:wave: @agbuckley, as the author of `tex2pix` (thanks! :bow:), can you comment on if there is any plans to maintain `tex2pix` in the future? Or would you recommend that we look for an alternative that is more probable to be patched if there are issues that are found. I can imagine that you already have more than enough on your plate and being responsible for maintaining another codebase might very understandably not be high on the list. :)

Discussion:
codecov[bot]: # [Codecov](https://codecov.io/gh/scikit-hep/pylhe/pull/131?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) Report
> Merging [#131](https://codecov.io/gh/scikit-hep/pylhe/pull/131?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) (27bf546) into [master](https://codecov.io/gh/scikit-hep/pylhe/commit/555f5d921231919904d99697748049911c86c8c1?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) (555f5d9) will **decrease** coverage by `5.46%`.
> The diff coverage is `n/a`.

> :exclamation: Current head 27bf546 differs from pull request most recent head e78101d. Consider uploading reports for the commit e78101d to get more accurate results

```diff
@@            Coverage Diff             @@
##           master     #131      +/-   ##
==========================================
- Coverage   62.82%   57.35%   -5.47%     
==========================================
  Files           2        2              
  Lines         234      204      -30     
  Branches       59       46      -13     
==========================================
- Hits          147      117      -30     
- Misses         76       80       +4     
+ Partials       11        7       -4     
```

| Flag | Coverage Δ | |
|---|---|---|
| unittests-3.10 | `?` | |
| unittests-3.6 | `57.35% <ø> (-2.91%)` | :arrow_down: |

Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep#carryforward-flags-in-the-pull-request-comment) to find out more.

| [Impacted Files](https://codecov.io/gh/scikit-hep/pylhe/pull/131?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep) | Coverage Δ | |
|---|---|---|
| [src/pylhe/\_\_init\_\_.py](https://codecov.io/gh/scikit-hep/pylhe/pull/131/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep#diff-c3JjL3B5bGhlL19faW5pdF9fLnB5) | `62.87% <ø> (-5.15%)` | :arrow_down: |
| [src/pylhe/awkward.py](https://codecov.io/gh/scikit-hep/pylhe/pull/131/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=scikit-hep#diff-c3JjL3B5bGhlL2F3a3dhcmQucHk=) | `32.43% <0.00%> (-2.71%)` | :arrow_down: |

------

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

eduardo-rodrigues: > I think the only thing that would be good to note/fix(?) for the future would be that the old method allowed for LaTeX fonts.

> Is there a way to get LaTeX fonts? This is somewhat steering back into the questions of Issue https://github.com/scikit-hep/pylhe/issues/53, which I haven't taken the time to review and refresh myself on, so apologies if all of this should be obvious.

It must be possible since both packages in the end write DOT language files. I used what I'm doing in DecayLanguage. I'm putting your request down as a to-do to look into this soon, after a few days off ... In the meantime, if this goes in we're good tp proceed with the other MRs dependent on this.



eduardo-rodrigues: Hello @matthewfeickert, I finally remembered to re-instate a test as you had before, see your comment https://github.com/scikit-hep/pylhe/pull/131#discussion_r901226407 above.

Coverage is to be worked on but that's not specific to this MR, rather a general issue that coverage is missing for certain classes, etc. That's clearly outsidethe scope of this MR. This is an important MR as it makes dependencies easier and unblocks some dependent MRs.

As for your comment https://github.com/scikit-hep/pylhe/pull/131#issuecomment-1162397488 I would gladly make this a follow-up issue as it is a quality of life improvement, and something I can look into in the future. Again, a different matter, second priority.

In short, I really think this is ready to do. If happy please approve and I will make the follow-up issue. Thanks a lot.
eduardo-rodrigues: Follow-up issue created - https://github.com/scikit-hep/pylhe/issues/135 for discussion https://github.com/scikit-hep/pylhe/pull/131#issuecomment-1162397488.
eduardo-rodrigues: Hi @matthewfeickert, I will merge this really shortly since the follow-up task is created and blocking this further is counter-productive and not helping anyone, certainly not the users. Coverage is a serious matter to deal with across the files, not just the 2-3 lines mentioned by codecov above ;-).
matthewfeickert: > Hi @matthewfeickert, I will merge this really shortly since the follow-up task is created and blocking this further is counter-productive and not helping anyone, certainly not the users. Coverage is a serious matter to deal with across the files, not just the 2-3 lines mentioned by codecov above ;-).

Okay. :+1: I still have some concerns about trading one viz library for another that now changes how the visualizations look, but I'm willing to kick that down the road a bit to sometime when I have more time to do the work here for these concerns to matter.

Patch:
diff --git a/README.md b/README.md
index 0caa527..d56be5a 100644
--- a/README.md
+++ b/README.md
@@ -22,12 +22,6 @@ To install `pylhe` from PyPI you can just do
 python -m pip install pylhe
 ```
 
-and to get the required libraries to be able to visualize events install with the "viz" extra
-
-```
-python -m pip install pylhe[viz]
-```
-
 The visualization capabilities require external dependencies of [Graphviz](https://graphviz.org/) and LaTeX.
 
 ## Get started
diff --git a/setup.cfg b/setup.cfg
index 5359876..bced88f 100644
--- a/setup.cfg
+++ b/setup.cfg
@@ -33,9 +33,7 @@ packages = find:
 include_package_data = True
 python_requires = >=3.6
 install_requires =
-    networkx>=2.2
     graphviz>=0.12.0
-    tex2pix>=0.3
     particle>=0.16
     awkward>=1.2.0
     vector>=0.8.1
diff --git a/setup.py b/setup.py
index df20055..08a3fa4 100644
--- a/setup.py
+++ b/setup.py
@@ -2,18 +2,13 @@ from setuptools import setup
 
 extras_require = {}
 extras_require["lint"] = sorted({"flake8", "black"})
-# c.f. https://networkx.org/documentation/stable/install.html#optional-packages
-extras_require["viz"] = sorted({"pydot~=1.4", "dot2tex~=2.11"})
 extras_require["test"] = sorted(
-    set(
-        extras_require["viz"]
-        + [
-            "pytest~=6.0",
-            "pytest-cov>=2.5.1",
-            "scikit-hep-testdata>=0.3.1",
-            "pydocstyle",
-        ]
-    )
+    {
+        "pytest~=6.0",
+        "pytest-cov>=2.5.1",
+        "scikit-hep-testdata>=0.3.1",
+        "pydocstyle",
+    }
 )
 extras_require["develop"] = sorted(
     set(
diff --git a/src/pylhe/__init__.py b/src/pylhe/__init__.py
index a3ab03a..6b83fd6 100644
--- a/src/pylhe/__init__.py
+++ b/src/pylhe/__init__.py
@@ -1,11 +1,7 @@
 import gzip
-import os
-import subprocess
 import xml.etree.ElementTree as ET
 
 import graphviz
-import networkx as nx
-import tex2pix
 from particle import latex_to_html_name
 from particle.converters.bimap import DirectionalMaps
 
@@ -27,7 +23,6 @@ __all__ = [
     "read_num_events",
     "register_awkward",
     "to_awkward",
-    "visualize",
 ]
 
 
@@ -349,35 +344,3 @@ def read_num_events(filepath):
             element.tag == "event"
             for event, element in ET.iterparse(fileobj, events=["end"])
         )
-
-
-def visualize(event, outputname):
-    """
-    Create a PDF with a visualisation of the LHE event record as a directed graph
-    """
-
-    # retrieve mapping of PDG ID to particle name as LaTeX string
-    _PDGID2LaTeXNameMap, _ = DirectionalMaps(
-        "PDGID", "LATEXNAME", converters=(int, str)
-    )
-
-    g = nx.DiGraph()
-    for i, p in enumerate(event.particles):
-        g.add_node(i, attr_dict=p.__dict__)
-        try:
-            iid = int(p.id)
-            name = _PDGID2LaTeXNameMap[iid]
-            texlbl = f"${name}$"
-        except KeyError:
-            texlbl = str(int(p.id))
-        g.nodes[i].update(texlbl=texlbl)
-    for i, p in enumerate(event.particles):
-        for mom in p.mothers():
-            g.add_edge(event.particles.index(mom), i)
-    nx.nx_pydot.write_dot(g, "event.dot")
-
-    p = subprocess.Popen(["dot2tex", "event.dot"], stdout=subprocess.PIPE)
-    tex = p.stdout.read().decode()
-    tex2pix.Renderer(tex).mkpdf(outputname)
-    subprocess.check_call(["pdfcrop", outputname, outputname])
-    os.remove("event.dot")


Test patch:
diff --git a/tests/test_api.py b/tests/test_api.py
index 1e3dab4..d3b288f 100644
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -26,7 +26,6 @@ def test_top_level_api():
         "read_num_events",
         "register_awkward",
         "to_awkward",
-        "visualize",
     ]
 
 
diff --git a/tests/test_visualize.py b/tests/test_visualize.py
index c78fcf0..48336e4 100644
--- a/tests/test_visualize.py
+++ b/tests/test_visualize.py
@@ -5,18 +5,7 @@ import skhep_testdata
 import pylhe
 
 
-def test_visualize(tmpdir):
-    lhe_file = skhep_testdata.data_path("pylhe-testfile-pr29.lhe")
-    events = pylhe.read_lhe_with_attributes(lhe_file)
-
-    start_event = 1
-    stop_event = 2
-    filename = tmpdir.join(f"event{start_event}.pdf")
-    for event in itertools.islice(events, start_event, stop_event):
-        pylhe.visualize(event, filename)
-
-
-def test_LHEEvent_graph():
+def test_LHEEvent_graph_source():
     lhe_file = skhep_testdata.data_path("pylhe-testfile-pr29.lhe")
     events = pylhe.read_lhe_with_attributes(lhe_file)
 
@@ -26,3 +15,11 @@ def test_LHEEvent_graph():
     # pi unicode charecter is '&#x03c0;'
     assert event.graph.source.count("&#x03c0;") == 8
     assert "<td>p</td>" in event.graph.source
+
+
+def test_LHEEvent_graph_render():
+    lhe_file = skhep_testdata.data_path("pylhe-testfile-pr29.lhe")
+    events = pylhe.read_lhe_with_attributes(lhe_file)
+
+    event = next(itertools.islice(events, 1, 2))
+    event.graph.render(filename="test_event1", format="pdf", cleanup=True)