Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[feature] ability to import external yaml files for sharing connector and cable definitions #220

Open
gmkado opened this issue Mar 4, 2021 · 26 comments

Comments

@gmkado
Copy link

gmkado commented Mar 4, 2021

I would love to be able to have all my connector definitions in a single file and include it into my cable drawing yamls. This would allow for things like a shared "library" of connectors that can be shared across a company.

From my searches online there are ways to do this with pyyaml:

I tried adding this code to the wireviz.py dev build but keep running into yaml syntax errors. My knowledge of yaml parsing is pretty limited.

@kvid
Copy link
Collaborator

kvid commented Mar 5, 2021

Thank you for your effort, but please also try including your library using the existing command line option --prepend-file. Does that solve your issue? If not, please describe use cases where more or different features are required.

@gmkado
Copy link
Author

gmkado commented Mar 9, 2021

oops, should have read the docs better. Thanks this does the trick!

@gmkado gmkado closed this as completed Mar 9, 2021
@gmkado
Copy link
Author

gmkado commented Mar 12, 2021

Hmm I'm going to re-open this because the --prepend-file leaves something to be desired. Ideally you would be able to keep common connector and cable definitions in a separate folder and point to it. But doing this breaks paths to image files referenced by the definitions.

@kvid
Copy link
Collaborator

kvid commented Mar 12, 2021

Hmm I'm going to re-open this because the --prepend-file leaves something to be desired. Ideally you would be able to keep common connector and cable definitions in a separate folder and point to it. But doing this breaks paths to image files referenced by the definitions.

The PR #189 already implements relative image paths differently, but your use case is still an issue and I describe the root cause in #189 (comment).

@formatc1702
Copy link
Collaborator

There has been some progress in #189 regarding this issue, @gmkado please have a look and feel free to post feedback.

formatc1702 added a commit that referenced this issue Oct 7, 2021
formatc1702 added a commit that referenced this issue Oct 8, 2021
@liambeguin
Copy link

What about having something like this:

templates:
  includes:
    - includes/DB9.yaml
    - includes/molex.yaml
    - include/inline_headers.yaml

This would make it explicit that this specific harness file depends on a list of "libraries"

Thanks for your work on this tool! It's really great at making harness drawings more efficient!

@drid
Copy link

drid commented May 15, 2024

What about having something like this:

templates:
  includes:
    - includes/DB9.yaml
    - includes/molex.yaml
    - include/inline_headers.yaml

This would make it explicit that this specific harness file depends on a list of "libraries"

Thanks for your work on this tool! It's really great at making harness drawings more efficient!

I agree with that format, this would also make preview with vscode plugin possible.

@kvid
Copy link
Collaborator

kvid commented May 17, 2024

The YAML parser library we currently use seems to need the full YAML contents as one long string before beginning the parsing. That's why it's a bit hard to use YAML for specifying more YAML files to include. I don't know if the suggested #223 might help?

@martinrieder
Copy link
Contributor

martinrieder commented May 21, 2024

@kvid take a look at https://github.com/adobe/himl and https://github.com/zerwes/hiyapyco

The hierarchical loading of YAML files could happen before the parsing in any other library, even the current one.

I still find the concept of using include statements quite promising. Since YAML uses # to start comments, you could theorerically employ any C code preprocessor with:

#include "templates.yml"

If it wasn't for the Windows compatibility, the beginning of a YAML file could even carry a shebang statement like:

#!/usr/bin/wireviz --prepend-file xyz.yml

This could turn your YAML file into an executable script!

Both would also propose a solution to #19.
This also relates to the CLI change suggested in #238.

@tobiasfalk
Copy link

@martinrieder what is the Problem with windows, is this something purely becaus of the YAML library or what?

@tobiasfalk
Copy link

One way could be to do it like/with a preprocessor, that goes through the file befor it is treated like a YAML file.

@martinrieder
Copy link
Contributor

martinrieder commented May 21, 2024

@tobiasfalk see here: https://realpython.com/python-shebang/

The Windows CMD processor does not recognize this statement, but there are workarounds. This method would also rely on STDIN/STDOUT, and which is WIP on #320.

Having the possibility to store CLI arguments in the YAML file would be a great way to generate reproducible output. We should place this header before an eventual --- block start, so it could be processed before any YAML code.

I could come up with a proposal that incorporates #307 and #348.

@tobiasfalk
Copy link

tobiasfalk commented May 21, 2024

@martinrieder Then why not have the input file be a .wz(WireViz) file, this way one can add the association for Windows and the first line for Unix/Linux systems.
Then WiereViz takes this file let's a Preprocessor run over it, wich add all the includes and so, and exports a pure YAML file(.wz.yalm) wich then runs through the current code.

Similar to how a C compiler does it.

@martinrieder
Copy link
Contributor

martinrieder commented Jun 6, 2024

@tobiasfalk @kvid I suggested Jinja in #377 for the HTML output. It could be applied to the input YAML as well!

See the topic on template inheritance from the docs. And here for some examples specific to YAML.

Just a side note: have a look at https://github.com/trailofbits/graphtage, which can be used to diff and merge various files like yaml.

@tobiasfalk
Copy link

@martinrieder yes would also work, personally I would prefer C/C++ style, since I am familiar with it

@martinrieder
Copy link
Contributor

martinrieder commented Jun 9, 2024

@kvid wrote:

The YAML parser library we currently use seems to need the full YAML contents as one long string before beginning the parsing. That's why it's a bit hard to use YAML for specifying more YAML files to include. I don't know if the suggested #223 might help?

@gmkado @tobiasfalk
What do you think about the YAML syntax that LamaAni/bole suggests?

settings:
    inherit: False # If true, allow inherit parent folders.
    inherit_siblings: True # If true allow inherit configuration files in the same source directory.
    allow_imports: True # If false dose not allow imports.
    use_deep_merge: True # Merge configurations via deep merge. If false, Only root keys are merged (and overwritten)
    concatenate_lists: True # When merging, append the merged list to the current one.
imports:
    - "**/*.config.yaml" # Recursively import all .config.yaml
    - path: my-config.yaml
      required: False # this item is not required.
      recursive: null # Applies if glob. Import recursively.
environments:
    [env name]:{ config overrides (any)}

It comes really close to what was suggested earlier by @drid and @liambeguin in #220 (comment)

@liambeguin
Copy link

It's been a while since I posted that comment, and looking at it again, I think going with Jinja2 would be great! It provides a lot more than just includes and is also something pretty standard people are used to.

I implemented a proof-of-concept here: #382

@martinrieder
Copy link
Contributor

martinrieder commented Jun 10, 2024

Nice. Thanks for taking the initiative, @liambeguin! I will give it a try ASAP...
I would strongly recommend using the # symbol instead of (or additionally) curly brackets. If Jinja statements look like a comment, you could still parse it as an ordinary YAML file. This would allow #348 to work without Jinja as well.

@tobiasfalk
Copy link

Yes looks good.
As mentioned earlier, could we change the file ending in the same move?
Would not only the things mentioned above but programs like VScode could also detect dem as a wz file and have the right autocompletion and so on.

@liambeguin
Copy link

I would strongly recommend using the # symbol instead of (or additionally) curly brackets.

I'm not sure I understand what you mean by that. do you have an example?

Jinja is a preprocessor, so if it sees it's keywords, it will interpret them otherwise the file is left untouched and can be parsed just like before.

This would allow #348 to work without Jinja as well.

Schema validation is a great idea! have you considered using pydantic for this?

@martinrieder
Copy link
Contributor

@tobiasfalk please stick to .yaml or .yml as a file extenstion, so that text editors will properly recognize it. You can surely configure them to use any other extension, but it seems inconvenient.
We might eventually upload a YAML schema to the Schema Store, which could indicate a file name pattern. Common practice here is to use something like .wireviz.yaml as a file extension.

@martinrieder
Copy link
Contributor

martinrieder commented Jun 10, 2024

I would strongly recommend using the # symbol instead of (or additionally) curly brackets.

@liambeguin wrote:
I'm not sure I understand what you mean by that. do you have an example?

https://jinja.palletsprojects.com/en/3.1.x/api/#jinja2.Environment

  • block_start_string The string marking the beginning of a block.
    • Defaults to {%, replaced by #%.
  • block_end_string The string marking the end of a block.
    • Defaults to %}, replaced by %#.
  • variable_start_string The string marking the beginning of a print statement.
    • Defaults to {{, replaced by #{.
  • variable_end_string The string marking the end of a print statement.
    • Defaults to }}, replaced by }#.
  • comment_start_string The string marking the beginning of a comment.
    • Defaults to {#, replaced by ##.
  • comment_end_string The string marking the end of a comment.
    • Defaults to #}, replaced by ##.

@liambeguin wrote:

Jinja is a preprocessor, so if it sees it's keywords, it will interpret them otherwise the file is left untouched and can be parsed just like before.

Understood, but I am afraid that it would confuse syntax highlighting when editing these templates. The hash symbol will make Jinja statements look like YAML comments.

This would allow #348 to work without Jinja as well.

Schema validation is a great idea! have you considered using pydantic for this?

Thanks for the hint on Pydantic. I should put this on the list, though maybe into #223. Have another look at #348: I have just uploaded the YAML schema for Yamale.

@liambeguin
Copy link

@martinrieder oh I see. I mean why not, but not all lines will be starting with a comment char. the content of blocks will not make a lot of sense if you don't consider the jinja code.

If you concern is just for code highlighting, parsers that understand jinja usually know that it's a preprocessor and allow for a 2 level highlighting.

Thanks for the hint on Pydantic. I should put this on the list, though maybe into #223. Have another look at #348: I have just uploaded the YAML schema for Yamale.

I didn't know about yamale, interesting.. I usually just do Ruamel + jsonschema, but more recently pydantic has removed the need for manual schema handling.

@martinrieder
Copy link
Contributor

@liambeguin I was really happy to see your PR with the Jinja integration. It provides some great possibilities to generate dynamic content, however I would consider it an advanced feature.

I've been looking for a simple way to do inclusion of YAML files that would not require learning a new syntax. I found that the most intuitive solution is ingesting directory trees of YAML files. Users would not need to learn about Jinja syntax if only static content is required. This would also not exclude using Jinja!

Please see the following implementation in Go that could be employed as an external tool.
Divvy Yaml - dy parses a directory tree according to the following rules:

  • A directory is a text key
  • A file name has contents that are rendered under a key named after the file prefix
  • A file name that begins with an underscore is rendered without a key at the current indentation level

If a Python implementation would be required, then YAML Tree might provide an alternative. There is also Directory-Mapper, which provides a lot more functionality.

Please have a look at those tools and tell me what you think. How could it be combined with the Jinja implementation?

@liambeguin
Copy link

@martinrieder I like the approach of having a directory structure interpreted as YAML. I think it would be great to load a library of connectors and wires for example, but I'd keep it limited to that (that lib could also be a part of the repo, allowing to reuse and share many basic components). I think I mentioned it earlier, but I'd also recommend having a single way of including files in the yaml syntax. I'd have the yaml tree loader be a part of the code, with the option of passing in additional paths from the command line (to load a custom library for example).

@martinrieder
Copy link
Contributor

martinrieder commented Jul 2, 2024

@liambeguin wrote:

I think I mentioned it earlier, but I'd also recommend having a single way of including files in the yaml syntax.

Agreed, though implementing the include statement directly in the YAML syntax can create all kinds of difficulties in the parsing process. Just have a look at the following example and you will recognize that there is more complexity to it than simply including a single file.

What do you think about the YAML syntax that LamaAni/bole suggests?

settings:
    inherit: False # If true, allow inherit parent folders.
    inherit_siblings: True # If true allow inherit configuration files in the same source directory.
    allow_imports: True # If false dose not allow imports.
    use_deep_merge: True # Merge configurations via deep merge. If false, Only root keys are merged (and overwritten)
    concatenate_lists: True # When merging, append the merged list to the current one.
imports:
    - "**/*.config.yaml" # Recursively import all .config.yaml
    - path: my-config.yaml
      required: False # this item is not required.
      recursive: null # Applies if glob. Import recursively.
environments:
    [env name]:{ config overrides (any)}

So, including would require a preprocessing step that merges everything into a monolithic YAML document. You can quickly run into conflicts with duplicate definitions or recursion if it is not implemented properly.

I just now found this 4yo post discussing a similar issue in #19 (comment).

Do you have a recommendation on which library to use for this purpose? What do you think about the ones suggested above and in #223?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants