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

Unparsing map value by catn schema does not always keep entry positional order #925

Closed
radhikalism opened this issue Aug 5, 2023 · 2 comments · Fixed by #926 or #941
Closed

Unparsing map value by catn schema does not always keep entry positional order #925

radhikalism opened this issue Aug 5, 2023 · 2 comments · Fixed by #926 or #941

Comments

@radhikalism
Copy link
Contributor

radhikalism commented Aug 5, 2023

Steps:

  • Given some :catn schema C,
  • parse a valid sequence V into map M: (parse C V),
  • Then unparse map M into result sequence R: (unparse C M).

Expected: (is (= V R))

  • i.e. the roundtrip of parsing and unparsing V to M to R, should return the original sequence with items in the same order as the :catn definition.

Actual: (is (not= V R))

  • i.e. unparsing does not guarantee that the result sequence R will be in the same positional order of the original :catn schema C entries or of input sequence V.

Example at JVM Clojure REPL:

> (let [schema-8 [:catn
                  [:a :int]
                  [:b :int]
                  [:c :int]
                  [:d :int]
                  [:e :int]
                  [:f :int]
                  [:g :int]
                  [:h :int]]
        schema-9 (into schema-8 [[:i :int]])
        input-8 [1 2 3 4 5 6 7 8]
        input-9 (into input-8 [9])]
    {:roundtrip-8 (->> input-8
                       (m/parse schema-8)
                       (m/unparse schema-8))
     :roundtrip-9 (->> input-9
                       (m/parse schema-9)
                       (m/unparse schema-9))})
;; result:
{:roundtrip-8 [1 2 3 4 5 6 7 8],
 :roundtrip-9 [5 7 3 8 2 4 6 9 1]}

Workaround using :cat + :orn:

> (let [schema-8 [:cat
                  [:orn [:a :int]]
                  [:orn [:b :int]]
                  [:orn [:c :int]]
                  [:orn [:d :int]]
                  [:orn [:e :int]]
                  [:orn [:f :int]]
                  [:orn [:g :int]]
                  [:orn [:h :int]]]
        schema-9 (into schema-8 [[:orn [:i :int]]])
        input-8 [1 2 3 4 5 6 7 8]
        input-9 (into input-8 [9])]
    {:roundtrip-8 (->> input-8
                       (m/parse schema-8)
                       (m/unparse schema-8))
     :roundtrip-9 (->> input-9
                       (m/parse schema-9)
                       (m/unparse schema-9))})
;; result:
{:roundtrip-8 [1 2 3 4 5 6 7 8],
 :roundtrip-9 [1 2 3 4 5 6 7 8 9]}

Comments

It seems to depend on the arbitrary order of the hash map that typically varies by its size, which suggests unparse applies to the seq of entries of map M. So small maps misleadingly seem to hold that (is (= V R)) by coincidence.

Other seqexp schemas like :* or :altn seem to preserve the order of the input value to unparse (rather than schema entry definition order), so maybe the order lossiness of :catn parse-unparse roundtrips is actually consistent in principle and should be expected.

If so, then it would still be interesting to have some other builtin schema to achieve the expected roundtrip behavior of this issue. Maybe a named-tuple or ordered-map type of schema (:catns?) as a terser form of the workaround above returning a MapEntry sequence preserving order.

@ikitommi
Copy link
Member

ikitommi commented Aug 6, 2023

Thanks for the detailed issue. I think we can use the schema entry keys to sort the result in unparse.

ikitommi added a commit that referenced this issue Aug 8, 2023
@ikitommi ikitommi mentioned this issue Aug 8, 2023
@ikitommi
Copy link
Member

ikitommi commented Aug 8, 2023

Should be fixed in master.

@ikitommi ikitommi mentioned this issue Aug 31, 2023
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 a pull request may close this issue.

2 participants