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

[Bug] add missing import of embed_svg_images #363

Merged
merged 2 commits into from
Jun 7, 2024

Conversation

adaemmer
Copy link
Contributor

@adaemmer adaemmer commented May 24, 2024

Harness.py uses embed_svg_images without importing it first.

Edit:
wireviz 0.4
python 3.11.9
dot - graphviz version 11.0.0 (20240428.1522)

It occurs as issue if you embed wireviz in a python script

e.g.:

from wireviz.wireviz import parse

y = """---
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
"""
parse(y, return_types="svg", output_name="test.svg")
Traceback (most recent call last):
  File "C:\Users\adaemmer\git\test\wireviz_demo.py", line 32, in <module>
    parse(y, return_types="svg", output_name="test.svg")
  File "C:\Users\adaemmer\.virtualenvs\test\Lib\site-packages\wireviz\wireviz.py", line 395, in parse
    returns.append(harness.svg)
                   ^^^^^^^^^^^
  File "C:\Users\adaemmer\.virtualenvs\test\Lib\site-packages\wireviz\Harness.py", line 669, in svg
    return embed_svg_images(graph.pipe(format="svg").decode("utf-8"), Path.cwd())
           ^^^^^^^^^^^^^^^^
NameError: name 'embed_svg_images' is not defined. Did you mean: 'embed_svg_images_file'?

@kvid kvid mentioned this pull request May 25, 2024
25 tasks
@kvid kvid changed the base branch from master to release/v0.4.1-rc May 25, 2024 16:45
@kvid
Copy link
Collaborator

kvid commented May 26, 2024

@adaemmer - Thank's for reporting this. Do you have an example YAML input that triggers the error? Please also provide information about your set of versions, as described in the contributing guidelines.

@adaemmer adaemmer changed the title add missing import of embed_svg_images [Bug] add missing import of embed_svg_images May 27, 2024
@adaemmer
Copy link
Contributor Author

Thanks @kvid for pointing me to the contributing guidelines. I some how didn't see them before.

@kvid kvid added the bug Something isn't working label May 27, 2024
src/wireviz/Harness.py Outdated Show resolved Hide resolved
@kvid
Copy link
Collaborator

kvid commented Jun 4, 2024

@adaemmer wrote:

[...]

parse(y, return_types="svg", output_name="test.svg")

Be aware that the parse() function returns the requested output data (in a tuple if more than one return type), and that output_name should be without extension (in your case, this will mainly be used as name of temporary files).

svg_text = parse(y, return_types="svg", output_name="test")
assert svg_text

resort module import

Co-authored-by: kvid <[email protected]>
@adaemmer
Copy link
Contributor Author

adaemmer commented Jun 5, 2024

Be aware that the parse() function returns the requested output data (in a tuple if more than one return type), and that output_name should be without extension (in your case, this will mainly be used as name of temporary files).

I was just trying to write a simplified example, but yes of course you are right.

@kvid kvid merged commit 7191457 into wireviz:release/v0.4.1-rc Jun 7, 2024
2 checks passed
kvid added a commit that referenced this pull request Jun 7, 2024
Resort module import:

Co-authored-by: kvid <[email protected]>
kvid added a commit that referenced this pull request Jul 5, 2024
Resort module import:

Co-authored-by: kvid <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants