cognitect / transit-format

A data interchange format.
1.87k stars 36 forks source link

Is 'asMapKey' recursive? #21

Closed syslo closed 9 years ago

syslo commented 9 years ago

Reading JSON ["^ ", ["~#'", "cached"], 0, "~$Explain", "^0"] differs in JavaScript and Python implementations:

This is because transit-py forwards asMapKey to list items, while transit-js does not. It is not specified which behavior is correct.

One possible answer is that quoted value is composite and therefore it should'n be a key. But it would be nice to process even this case in the same way.

hleumas commented 9 years ago

Hi guys, is there any resolution to this question?

swannodette commented 9 years ago

We're looking into it.

jlouis commented 9 years ago

FWIW, isaiah/transit-erlang acts like transit-py in this regard. But it modeled a lot after the python implementation so it is pretty obvious why. Do note, interestingly, that there are no exemplar tests nor test.check tests which have been capturing this, which suggests the addition of a test to the exemplar suite to capture this problem and remove it from all implementations.

Something like transit-erlang is internally consistent, but it doesn't have tests that go cross-implementation so errors like these are not caught :/

swannodette commented 9 years ago

@jlouis yes once we sort out what the right behavior is we'll definitely get this converted into a test for all clients.

swannodette commented 9 years ago

I've cut a new release of transit-js that properly passes asMapKey recursively. We'll leave this ticket open until we get an exemplar catching this particular case.

hleumas commented 9 years ago

So, should I understand that asMapKey should be recursive?

swannodette commented 9 years ago

@hleumas I'm going to go out on a limb as say yes asMapKey should be considered recursive as that's how all the implementations work.

hleumas commented 9 years ago

@swannodette awesome, so this issue can be closed finally!