WireViz 0.3: file_out argument to wireviz.parse() became mandatory

[amotl]

Hi Daniel,

as a followup to #252, while working on wireviz/wireviz-web#3, I would like to bring another detail to your attention.

Specifically, it revolves around the patch wireviz/wireviz-web@e9148b49 we needed to satisfy the new mandatory requirement of wireviz.parse() to obtain the file_out argument.

As the code is focused on getting a full Harness instance back by signalling return_types="harness", it was a bit surprising that this argument became mandatory, given that it is announced as optional in the function signature. Fair enough, no worries about the breakage per se, we are playing 0.x releases here.

However, I would like to ask / propose if it would be worth making this argument optional again? Following the code, I discovered only two places where it is actually used: [1] and [2]. The first occurence uses the filename to deduce a title. The second one forwards it as gv_dir to an instance of Image. While the second one honors Noneness through file_out if file_out else '', the first one Path(file_out).stem does not.

Instead, it croaks with

Traceback (most recent call last):
  File "/Users/amo/dev/daq-tools/sources/wireviz-web/wireviz_web/core.py", line 119, in wireviz_render
    harness: Harness = wireviz.parse(yaml_input=input_yaml, return_types="harness")
  File "/Users/amo/Library/Caches/pypoetry/virtualenvs/wireviz-web-UCc3rrkF-py3.9/lib/python3.9/site-packages/wireviz/wireviz.py", line 44, in parse
    harness.metadata['title'] = Path(file_out).stem
  File "/usr/local/Cellar/[email protected]/3.9.7/Frameworks/Python.framework/Versions/3.9/lib/python3.9/pathlib.py", line 1082, in __new__
    self = cls._from_parts(args, init=False)
  File "/usr/local/Cellar/[email protected]/3.9.7/Frameworks/Python.framework/Versions/3.9/lib/python3.9/pathlib.py", line 707, in _from_parts
    drv, root, parts = self._parse_args(args)
  File "/usr/local/Cellar/[email protected]/3.9.7/Frameworks/Python.framework/Versions/3.9/lib/python3.9/pathlib.py", line 691, in _parse_args
    a = os.fspath(a)
TypeError: expected str, bytes or os.PathLike object, not NoneType

With kind regards,
Andreas.

[1] https://github.com/formatc1702/WireViz/blob/v0.3/src/wireviz/wireviz.py#L44
[2] https://github.com/formatc1702/WireViz/blob/v0.3/src/wireviz/wireviz.py#L57

[amotl]

We just submitted #254, which would solve the problem for us.

[formatc1702]

Thanks for bringing this to our attention!

I will openly admit that loading WireViz as a module as opposed to running from CLI has not seen the love it deserves. Please see my comment in #231 and the envisioned solution for the future:

Have a look at wireviz.py and its _get_yaml_data_and_path() function in the latest branch*, and a further improved version in the refactor/big-refactor branch, where file output and function return types are [or should be in the end, anyway] cleanly separated.

* The implementation in latest will likely never see the light of day since it will be overridden by the refactor/big-refactor before a new release.