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

Quote properties on edges too #40

Merged
merged 3 commits into from
Sep 13, 2023
Merged

Conversation

gabriel-fallen
Copy link
Contributor

When serializing in the DOT format MetaGraph (savedot function) was putting quotes around (non-XML) property values for vertices but not for edges of a graph. That broke my code when I tried to put labels containing several words on edges of my graph. So I went on and made serialization quote everything save for XML/HTML value in closer accordance with DOT language spec.

@gabriel-fallen
Copy link
Contributor Author

@matbesancon @simonschoelly hi guys! I'm sorry to bother you but could anybody please look at my PR? 😅

@filchristou
Copy link
Contributor

( I must say I do not have any rights/permissions to to accept this PR or whatever, but I am happy to help )

src/persistence.jl Show resolved Hide resolved
test/diagram_ref.dot Show resolved Hide resolved
test/diagram_ref.dot Show resolved Hide resolved
test/diagram_ref.dot Show resolved Hide resolved
test/dotformat.jl Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Sep 13, 2023

Codecov Report

Merging #40 (d9b89d5) into master (01d4dac) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master      #40   +/-   ##
=======================================
  Coverage   98.69%   98.69%           
=======================================
  Files           5        5           
  Lines         382      384    +2     
=======================================
+ Hits          377      379    +2     
  Misses          5        5           
Files Changed Coverage Δ
src/persistence.jl 95.65% <100.00%> (+0.19%) ⬆️

@gdalle gdalle merged commit 42a2f26 into JuliaGraphs:master Sep 13, 2023
6 checks passed
@gabriel-fallen
Copy link
Contributor Author

@gdalle thanks for merging, but above all thanks for your work on this library! 😃

@gdalle
Copy link
Member

gdalle commented Sep 14, 2023

You're welcome! Could you maybe try this v0.8.0 with your own code, and see if you can spot any other issues you'd like to address? Since it's a breaking release, if there's more low-hanging fruit I want it

@gabriel-fallen
Copy link
Contributor Author

@gdalle unfortunately, I don't work on that codebase currently (but I might get back to it next year). Anyway, it was only a very simple supplementary visualization to double-check the connectivity of the system, and the quoted labels were all we needed at that point. I don't remember even really using HTML there. Though, maybe I stuck <strong></strong> somewhere for clarity...

@gdalle
Copy link
Member

gdalle commented Sep 14, 2023

Alright then. I'll leave it dormant for some time and if no other issue pops up this month I'll release

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

Successfully merging this pull request may close these issues.

3 participants