[feature] Read from stdin and write output (PNG, SVG) to stdout

[ggrossetie]

Hey!

Would it be possible to support the following usage from the CLI:

$ cat input.yml
connectors:
  X1:
    type: D-Sub
    subtype: female
    pinlabels: [DCD, RX, TX, DTR, GND, DSR, RTS, CTS, RI]
  X2:
    type: Molex KK 254
    subtype: female
    pinlabels: [GND, RX, TX]

cables:
  W1:
    gauge: 0.25 mm2
    length: 0.2
    color_code: DIN
    wirecount: 3
    shield: true

connections:
  -
    - X1: [5,2,3]
    - W1: [1,2,3]
    - X2: [1,3,2]
  -
    - X1: 5
    - W1: s

$ cat input.yml | wireviz -f svg -

The above command will read the YAML file from the stdin and generate/write an SVG to stdout.

I can submit a pull request to implement this feature.

[kvid]

Thank you for the suggested functionality. A lot of new code has just recently been merged into the dev branch, and I don't know yet if something similar is already included. Maybe @formatc1702 can comment that?

It seems you intend to use a single input file named "-" as a flag to read from STDIN. What will happen if a set of input files are specified, and one of these is named "-"? Is it then possible to read STDIN for that single file while processing the other input files normally?

Independently from my question above, what should be the condition for writing an output file to STDOUT?

• Is it only when also reading from STDIN?
• What should happen if more than one output format is specified?
• What if the output filename (stem) is specified as "-" for a normal input file?
• What if the output filename is specified as something else for an input file named "-"?

Have you checked how well known Unix CLI tools (also able to process multiple input files) handle the conditions I describe above?

[ggrossetie]

What will happen if a set of input files are specified, and one of these is named "-"

As far as I know, you should not use a file named - unless you are looking for troubles.

$ vim -
Vim : Reading stdin...

$ echo 'hi' > -

$ cat -

The above command will wait for stdin 😱

I think we can use the same behavior as cat:

$ cat test.txt 
hi

$ cat test.txt  test1.txt 
hi
bonjour

stdin ignored if a file is provided:

$ echo 'hello' | cat test.txt
hi

$ echo 'hello' | cat 
hello

$ echo 'hello' | cat -
hello

Read the file then read from stdin

$ echo 'hello' | cat test.txt -
hi
hello

Is it only when also reading from STDIN?

By default, we should output to stdout when - is specified. We can also use the - convention to output to stdout explicitly:

-o -

The above option means output to - (stdout)

What should happen if more than one output format is specified?

I guess we should output them to stdout in the order they are specified.

What if the output filename (stem) is specified as "-" for a normal input file?

You mean:

$ wireviz test.yml -f svg -o -

The above command will read test.yml and output to stdout.

What if the output filename is specified as something else for an input file named "-"?

Since an input file should not be named -, you can do either:

Read from stdin, output to out.svg

cat input.yml | $ wireviz -f svg -o out.svg -

Read from stdin (empty), read from input.yml, output to out.svg

$ wireviz input.yml -f svg -o out.svg -

[formatc1702]

I have not considered this option, and as someone who is not a very heavy CLI user, I don't feel qualified to argue in favor or against such a feature. Feel free to play around with the dev branch (caution: currently very different from master) and submit a PR. This should not collide with the open to-do-s in dev.

[kvid]

I agree that when "-" as input file should be interpreted as STDIN, then "-" as output file
similarly should be interpreted as STDOUT. The output filename stem is by default equal to the input filename stem unless specified differently using -o.

I'm not sure if it makes sense to output more than one file format to STDOUT, but try it out to see if it can be usable.

There are a few places in the code where some warning text is written to STDOUT. Please make sure they are all written to STDERR to avoid possible conflicts with an output file written to STDOUT.

[ggrossetie]

In my opinion, the parse function is doing too much (from an API point of view).

WireViz/src/wireviz/wireviz.py

Lines 24 to 31 in 92af905

	def parse(
	inp: Union[Path, str, Dict],
	return_types: Union[None, str, Tuple[str]] = None,
	output_formats: Union[None, str, Tuple[str]] = None,
	output_dir: Union[str, Path] = None,
	output_name: Union[None, str] = None,
	image_paths: Union[Path, str, List] = [],
	) -> Any:

I think it would be better to handle the output part in another function.
The parse should always return a "result" object (currently named Harness) or an Error (if the parsing part went wrong).

def parse(
    input: Union[Path, str, Dict],
    image_paths: Union[Path, str, List] = [],
) -> ParseResult:

I would argue that image_paths should be included in an optional "options"/"context" object/dict:

def parse(
    input: Union[Path, str, Dict],
    options: dict,
) -> ParseResult:

Then, we could have functions on the ParseResult to export to any supported format. We could use the same terminology as GraphViz with pipe and save or use ``

parse_result.pipe(format="svg", encoding="utf8") # str
parse_result.pipe(format="png") # bytes
parse_result.save(filename) # str -> path where the file was saved 

[kvid]

Your arguments about the parse function seems reasonable and should also reduce the need for Any in type hints due to a cleaner API, but I am a bit worried that there might be some conflicts with the on-going work by @formatc1702 at PR #251. I'm currently at vacation and have not yet had time to try out any new code lately, including your PR #321. He already confirmed using STDIN and STDOUT should not conflict with his work-in-progress, but I hope he (or someone with more insight in #251 than me) also can comment the parse function changes you suggest.

[ggrossetie]

I think our goals are aligned:

• Shorter, more atomic functions
• More object-oriented (HTML tags, pins, wires, mates, colors, ...)

He already confirmed using STDIN and STDOUT should not conflict with his work-in-progress, but I hope he (or someone with more insight in #251 than me) also can comment the parse function changes you suggest.

For reference, I didn't include this change as part of #321

[kvid]

What happens if any connector or cable contains an image? Where should WireViz look for image files with relative paths when the YAML input is read from STDIN? Maybe #322 might help?

[ggrossetie]

We are using a image_paths array to resolve images. I guess we could add the current working directory os.getcwd().
Another idea is to add an option to be able to configure it.

Maybe #322 might help?

Indeed, embedding image as base64 is more portable since you don't rely on relative paths/external resources.

[ggrossetie]

@kvid @formatc1702 Hey! Is the dev branch (0.4.x) stable? I want to move forward and produce a native image (single binary) of WireViz (0.4.x) using Nuitka. My goal is to use this binary on https://kroki.io. Thanks!

[kvid]

@ggrossetie wrote:

@kvid @formatc1702 Hey! Is the dev branch (0.4.x) stable?

To be honest, I don't know. I've not had time to check out all the new code merged into dev during my vacation. @formatc1702 has done a great job by joining a number of old PR branches, started using isort & black, and also has a large refactoring PR to be added on top of that, but he has asked for help to finalize some remaining issues. We might need some time to get familiar with these remaining issues. See my questions in #316 (comment) - maybe @amotl has gained some insight about this matter?

What I do know, is that all commits to resolve change requests of the old PRs he merged into dev this summer, is added to his refactoring PR that is not yet merged in. I assume the original plan was to merge in all these PRs at the same time, but as this process has been much more time consuming than expected, this has unfortunately blocked all progress of other PRs and releases.

I want to move forward and produce a native image (single binary) of WireViz (0.4.x) using Nuitka. My goal is to use this binary on https://kroki.io. Thanks!

If you are in a hurry and need something stable, then I guess it's safer to use v0.3.2 as a base. Is that possible for you, or do you need some new features that currently are only in the dev branch?

[ggrossetie]

Thanks for your input.

(...) , but as this process has been much more time consuming than expected, this has unfortunately blocked all progress of other PRs and releases.

Let me know if I can help.

If you are in a hurry and need something stable, then I guess it's safer to use v0.3.2 as a base. Is that possible for you, or do you need some new features that currently are only in the dev branch?

I think I should be able to backport #321 on the 0.3.2 release.

[kvid]

(...) , but as this process has been much more time consuming than expected, this has unfortunately blocked all progress of other PRs and releases.

Let me know if I can help.

Thank you for offering your help!

Any kind of help is highly appreciated, e.g. testing new code against existing example inputs (and new test cases when needed), inspecting code to identify edge cases and/or code improvements, detect missing/confusing/wrong documentation, etc.

Be aware that #251 contains a lot of commits (I found it a bit overwhelming to go through each of them), but just looking at Files changed is perhaps a better approach. If you are willing to checkout the refactor/big-refactor branch and help identifying issues that need improvements, then I suggest classifying them into e.g.

1. Errors that must be fixed before a new release.
2. Low-hanging fruits of improvements that should be considered.
3. Nice-to-have improvements that can wait until a later release.
4. Strange or confusing code/comment/documentation that is hard to understand.

You can make comments at any changed code line, suggest improvements, and/or write a summary comment. You can choose to connect such comments into a review with a final summary comment, or just write independent comments.

See also #316 - where we discuss the the process of finalizing these PRs.

If you are in a hurry and need something stable, then I guess it's safer to use v0.3.2 as a base. Is that possible for you, or do you need some new features that currently are only in the dev branch?

I think I should be able to backport #321 on the 0.3.2 release.

I suggest a new PR (with master as base branch) to avoid throwing away your current PR code that might be needed when #251 is finalized, unless a later merging with your back-ported code in master is trivial.