-
-
Notifications
You must be signed in to change notification settings - Fork 50
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
save params in svg as json #344
base: master
Are you sure you want to change the base?
Conversation
f6dc35c
to
1de5823
Compare
The clash of double quotes having special meaning in both json and xml is highly annoying. I'm not sure I'm sold here though because a Though I don't hold my opinion strongly. I think a use case where that simple string replace wouldn't work would change my tune pretty quick :) |
I'm not sure I'm following. What would that string replace do? |
Sorry that should replace single quotes with double quotes so the value can be read by eg something like this is what I was thinking of: >>> import json
>>> import xml.etree.ElementTree as et
>>>
>>> json.loads(et.parse(open("sample.svg")).getroot().find(".//{http://purl.org/dc/elements/1.1/}source").text[20:].replace("'", '"'))
{'__seed__': 921181624, 'page_size': '4.5inx6.25in', 'scale': 1.0, 'pen_width_mm': 0.25, 'width': 15.0, 'height': 11.0, 'stroke_width': 0.12, 'density': 1.8, 'fuel': 3.0, 'min_length': 0.0, 'frequency': 0.15} But this will surely only work if the parameters don't contain anything too strange. Ints, floats, bools, and strings should be fine, but if params can hold more complicated values (eg with some kind of nesting) then this will definitely not work. |
Ah okay, I see what you mean. I'm hesitant on this because it's relying on a coincidence (that python's dict dump format happens to resemble json) rather than using an established, standardized format.
bools won't work because python capitalizes their names and json is case-sensitive:
And then there's the issue of non-json-primitive objects, like you mentioned. Overall I'm not sure that it makes sense to prioritize human readability for metadata that's embedded in an image file. I'd prefer to have an easily parsed format and a tool that extracts it (which is what I was building in the first place 😊) An alternative that preserves human readability while making the output more easily parseable would be to emit the attr values in XML. We could create a new namespace rather than overloading That said, I'd still prefer json 😊 |
Yeah good points all around. I agree that putting things into xml attributes isn’t better than json. |
@abey79 this seems like an improvement. I’d you’d like to merge it that is great to me |
I'm just wondering if we should be worried with the saver thread and the main thread sharing this object? I haven't looked at the code. Is the
|
Good question. To test, I added a 10 second
the like button currently disables all controls until the completed event fires, at which point I don't think it matters any more?
|
whoops, guess I hit the wrong keyboard shortcut |
If I remember correctly, it does save the |
yep, exactly. Which is useful data to have but I think can be reconstructed from the param data and stored seed? I have some other thoughts about how to proceed but I need a day or two to get them gathered and written down. |
I think all save methods (like, |
I think there's two separate issues that need to be addressed: unifying the svg output codepaths; and reworking param saving. I've opened #345 to discuss the former, let's keep this issue focused on param saving. I've given this a lot of thought and I think the best compromise is to store the parameters as XML. It's readily machine parseable, more human readable than entity-encoded JSON, and as an extra bonus it's more semantically valid/valuable. We can still store them as JSON as well, but I think the most utility comes from leaning on XML. I'm not an expert and need to spend more time with the specs to understand what's really possible. I think we'd want to study the various namespaces to find appropriate elements or create a vsketch or vpype namespace, but I was thinking structurally structurally something like this: <svg ...>
<metadata>
<rdf:RDF>
<vsk:param name="margin" type="number" unit="mm">10</param>
<vsk:param name="text" type="string">Lorem ipsum dolor sit amet...</param>
<vsk:param name="show_layout" type="boolean">true</param>
<vsk:params format="json">e21hcmdpbjogMTAsIHRleHQ6ICJMb3JlbSBpcHN1bSBkb2xvciBzaXQgYW1ldC4uLiJ9Cg==</params>
</rdf:RDF>
</metadata>
</svg> The key things are that each parameter is its own element and the attributes give enough information to parse it. I also like the idea of having a separate element (like What do y'all think? If this sounds good I'm happy to get started on it. |
Description
This reworks #327 to store the parameters in json rather than a serialized python dict. This allows for safer parsing without needing to eval python. It also refactors the save thread to pass the entire sketch object in so the thread can handle all the parsing/generating logic locally.
The only potential downside is that the quote marks in json are entity encoded in the svg, so it's quite a bit less readable for humans.
Before:
After:
Personally I think this is a small price to pay for easier programmatic extraction but I'd like to hear what @tyehle thinks.
Checklist
mypy
returns no errorpytest --runslow
succeedsmake clean && make html
indocs/
)black
andisort
)