Lost meta information / why not *print-dup* in pr-str-fully?
razum2um opened this issue · 8 comments
I'm taking about this place https://github.com/jimpil/duratom/blob/master/src/clojure/duratom/utils.clj#L25
Given https://github.com/clojure/clojure/blob/ee3553362de9bc3bfd18d4b0b3381e3483c2a34c/src/clj/clojure/core.clj#L3672L3674 and according to https://clojuredocs.org/clojure.core/*print-dup* I understand that print-dup is preferred way to serialise anything in order to restore it later, and print-method is used for console printing, just human representation etc..
Meaningful differences:
user=> ((juxt identity (comp meta read-string)) (binding [*print-dup* true] (pr-str (with-meta {:data 1} {:meta 1}))))
["^#=(clojure.lang.PersistentArrayMap/create {:meta 1}) #=(clojure.lang.PersistentArrayMap/create {:data 1})" {:meta 1}]
user=> ((juxt identity (comp meta read-string)) (binding [*print-dup* false] (pr-str (with-meta {:data 1} {:meta 1}))))
["{:data 1}" nil]So duratom loses meta info by default. On the other hand print-dup string should be read-string not a edn/read-string.
I suspect introducing this will break compatibility with written files, and I understand that :rw gives an easy option to configure it, just wonder why such default? Only because of sticking with edn?
Maybe warn users about this as well / add example in README how to serialise via print-dup/restore with read-string?
Binding *print-dup* to true, produces those literals, as you've demonstrated. In order to read those back, you also need *read-eval* bound to true, which I don't think is a reasonable default. However, your question made me realise that there is a related issue here. Apart from the metadata being lost, duratom also looses types (by default). For example, a sorted-map will be serialised as a regular map. That definitely needs to be explained/warned, and an example should be provided in the README. I will do my best to update it by end-of-week. Many thanks for bringing this to my attention 👍
Hi there,
I've added support for sorted-map, sorted-set & clojure.lang.PersistentQueue. These are great candidates for use with atoms, and so starting with the next release these will be printed/read correctly (without losing the actual type).
On your metadata issue/concern/query, I've added some baked-in (magical) support for objects with metadata, but I would encourage you to read the new section(s) in the README, especially the one about metadata. In short, I feel the best way to handle this is outside of duratom.
If you have any further questions, and/or feedback, these are welcome. 😄
Kind regards
Have I mentioned I love this library?
This is an interesting thread, reminder that nippy support avoids these problems and brings many other benefits as well (smaller size on disk, faster serialization and deserialization). We have more data in nippy backed duratoms than you think (even if you think we have a lot of data in nippy backed duratoms), and couldn't be happier.
eval is scary - agreed not a good default.
Keep up the great work.
Hi @harold , thanks for your kind words - they made my day :).
@razum2um you can safely ignore my comment from yesterday - I wasn't thinking clearly...
duratom now natively supports metadata for the default EDN readers/writers. See the updated README section. I think i'll stick with this approach, because as far as I can see it shouldn't break existing programs - even though the default behaviour has changed. Moreover, the previous behaviour can be trivially reinstated.
As always feedback is most welcome ;)
@jimpil - if I understand correctly, this change won't affect us. Our libraries are surprisingly well factored and we call duratom/duratom in exactly one place and it looks like this:
(duratom/duratom :local-file
:file-path file-path
:init {}
:rw {:read nippy/thaw-from-file
:write nippy/freeze-to-file})
That is, we never use the default :rw. :)
If we do ever upgrade and encounter trouble, I'll let you know.
Have a great day.
🙇♂️