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

jsondecode should treat array-of-array as row or n-dimensional matrix #6

Closed
mtmiller opened this issue Jan 30, 2020 · 8 comments
Closed
Assignees
Labels
bug Something isn't working
Projects
Milestone

Comments

@mtmiller
Copy link
Contributor

In Matlab, when decoding a JSON array-of-arrays where all values are numeric, it returns a numeric matrix of appropriate nested dimension.

Examples

jsondecode('[1, 2, 3]')  % 1-d column vector
jsondecode('[[1, 2, 3]]')  % 1-d row vector
jsondecode('[[1, 2], [1, 2]]')  % 2-d matrix
jsondecode('[[[1, 2], [1, 2]], [[1, 2], [1, 2]]]')  % 3-d matrix

I guess that this falls under the "condensation" logic. If any entries aren't numeric or are of inconsistent shape or size, then it falls back to returning cell arrays of cell arrays.

@mtmiller mtmiller changed the title jsondecode treats array-of-array as row or n-dimensional matrix jsondecode should treat array-of-array as row or n-dimensional matrix Jan 30, 2020
@apjanke
Copy link
Owner

apjanke commented Jan 30, 2020

Yep, the condensation logic should do this. This will take a bit of work to fix.

Since the dimensionality of numeric arrays is unlikely to get large (how often do you see a hundred-dimension array?), I think this can be done in a separate condensation step in the jsondecode M-code wrapper instead of the __jsonstuff_jsondecode_oct_- oct-file. That'll be easier to write.

(I think? Or will we have to pay for expensive negative cases, where we have to search all the members of a cell array to determine whether it is in fact a condensable array-of-numeric-arrays? In that case, maybe the logic should live in the oct-file after all.)

@apjanke apjanke self-assigned this Jan 30, 2020
@apjanke apjanke added the bug Something isn't working label Jan 30, 2020
@apjanke apjanke added this to Needs triage in jsonstuff via automation Jan 30, 2020
@apjanke apjanke moved this from Needs triage to High priority in jsonstuff Jan 30, 2020
@apjanke apjanke added this to the 0.4.0 milestone Jan 30, 2020
@mtmiller
Copy link
Contributor Author

Yeah I think it would be a little wasteful, and probably slower, to return a cell only to examine its dimensions and types and see if it can be collapsed into a matrix.

@apjanke
Copy link
Owner

apjanke commented Jan 30, 2020

Wasteful, but easier to write: from what I can figure, doing the condensation means a recursive depth-first traversal through the object graph of the decoded JSON data, condensing as you go, and then doing a permute() and cat() in a post-order manner at each node. Easy to write in M-code; I'm still trying to figure out how I'd approach it in C++.

I've checked in an M-code implementation at 511404d. Please have a look and let me know what you think. After we've established correctness, then let's see about pushing this logic down into the oct-file for performance.

@mtmiller
Copy link
Contributor Author

Cool, works for me for most examples of full higher order arrays.

Fails on some simpler and degenerate cases, for example

>> jsondecode ('[[1, 2]]')
ans =

   1
   2

should return a row vector. And empty or null arrays evidently aren't condensed, for example

>> jsondecode ('[[]]')
ans = [](0x0)

should return a cell array with an empty double array in it. Only gets condensed to a scalar if the innermost array consists of a scalar value.

Same with arrays of objects, [[[{}]]] should not get condensed (after fixing #9), but [[[{"a":1}]]] should (and does now) get condensed to a scalar struct.

@apjanke
Copy link
Owner

apjanke commented Jan 30, 2020

Argh, who designed the behavior for this stuff?

Okay, I'll special-case these caes.

@apjanke
Copy link
Owner

apjanke commented Feb 18, 2020

Okay. I pulled the condensation logic up into M-code until we can get the algorithm fully sorted out. (Leaving just the array-of-numerics-to-numeric-vector case in C++ for speed.) de8ec86

This made things clearer: Every level of [...] nesting around numeric or struct arrays causes them to get permuted/"rotated" one dimension higher. And then condensation is simply cat-ing the children along dimension 1 if they are conformant. (And always leaving char arrays as they are.)

Tests are passing now, and I think I've got all the cases you covered here. Closing this as fixed, and leaving #10 as a reminder to optimize this in C++ once we're sure we know what we're doing.

@apjanke apjanke closed this as completed Feb 18, 2020
jsonstuff automation moved this from High priority to Closed Feb 18, 2020
@mtmiller
Copy link
Contributor Author

Cool! I'll try this in a little bit

@mtmiller
Copy link
Contributor Author

Works great for me!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
jsonstuff
  
Closed
Development

No branches or pull requests

2 participants