patchGNU Octave - Patches: patch #9980, JSON encoder and decoder,...

 
 

patch #9980: JSON encoder and decoder, alternative to object2json

Submitter:  None
Submitted:  Thu 15 Oct 2020 06:04:42 PM UTC
   
 
Category:  Forge : new function Priority:  3 - Low
Status:  Done Privacy:  Public
Assigned to:  philipnienhuis Originator Email:  -email is unavailable-
Open/Closed:  Closed
* Mandatory Fields

Add a New Comment Rich Markup
   

Jump to the original submission

Sun 02 Jan 2022 03:41:36 PM UTC, comment #68: 


> Do you still tag releases in the package repository?

Thanks for the reminder. Yes I certainly planned to, but forgot.

I backed out the Java fixes, tagged the release, and reapplied. Not the most elegant, but the 2.6.4 tag in the repo should now be nearest to the state of release 2.6.4.
Only later it occurred to me that tagging could have been done in a simpler way. Due to consistent lack of time the last years I seem to have lost most of my mercurial proficiency.

Philip Nienhuis <philipnienhuis>
Group Member
Sun 02 Jan 2022 01:33:38 PM UTC, comment #67: 

Thanks. I pushed your patch to the release branch here:
https://hg.octave.org/mxe-octave/rev/fc663bd5f260

Btw: Do you still tag releases in the package repository? Or is that information stored only somewhere else now?

Markus Mützel <mmuetzel>
Group administrator
Sat 01 Jan 2022 09:35:19 PM UTC, comment #66: 

New patch w. dist-files.mk addition

(file #52596)

Philip Nienhuis <philipnienhuis>
Group Member
Sat 01 Jan 2022 08:34:33 PM UTC, comment #65: 

Ah yes sorry, I'll do.
Should it also be added to index.html? (I guess not)

Philip Nienhuis <philipnienhuis>
Group Member
Sat 01 Jan 2022 07:24:23 PM UTC, comment #64: 

Please, also add the new file to `dist-files.mk`.

Markus Mützel <mmuetzel>
Group administrator
Sat 01 Jan 2022 07:21:13 PM UTC, comment #63: 

Java BIST fixed here:
http://hg.code.sf.net/p/octave/io/rev/5a3553396bea

The relevant cset can be used in mxe-octave (just verified), can you push the derived mxe-octave cset (attached) please?


(file #52594)

Philip Nienhuis <philipnienhuis>
Group Member
Sat 01 Jan 2022 12:39:24 PM UTC, comment #62: 

Hmmm AFAIR I had fixed something along that line here:
http://hg.code.sf.net/p/octave/io/rev/83b2f81a28bf

... but for the last updates I used versions uploaded by Ketan that may not have had these fixes.  TBH I didn't check the 'hg diff'-s all the way into the 152 BIST tests ...
<... checking ...>
And you're right, they've fallen out :-(
Thanks for noting this.

I'll re-apply.

BTW my main aim for io-2.6.4 was to ensure that io still works with recent dependencies and current and esp. dev Octave, the to/fromJSON functions just fitted in nicely into this scheme.
A next io release isn't due very soon; I have several pending improvements elsewhere in the package but they're not so urgent.

Philip Nienhuis <philipnienhuis>
Group Member
Sat 01 Jan 2022 11:26:09 AM UTC, comment #61: 

When running the test suite on the recently released io package (at least it appeared on the package index), I get the following two failing tests:

>>>>> processing D:\SVN\Octave\test\octave-2022-01-01-11-19-w64 stable io-2.6.4\octave-2022-01-01-11-19-w64\mingw64\share\octave\packages\io-2.6.4\toJSON.m
***** test  ## unknown class
 obj = javaObject ("java.math.BigDecimal", 1.0);
  assert(toJSON(obj), '"[java.math.BigDecimal]"');
!!!!! test failed
unable to find Java Runtime Environment: software\javasoft\java runtime environment::CurrentVersion
***** test  ## struct with java object
 obj = javaObject ("java.math.BigDecimal", 1.0);
  assert(toJSON(struct('a',obj)), '{"a":"[java.math.BigDecimal]"}');
!!!!! test failed
unable to find Java Runtime Environment: software\javasoft\java runtime environment::CurrentVersion


That test should probably be guarded with a check for a working Java Runtime again. (See, e.g. 1cd610143a92)

Markus Mützel <mmuetzel>
Group administrator
Wed 29 Dec 2021 11:11:02 PM UTC, comment #60: 

Unfortunately I pushed the previous fromJSON.m (apologies).
Corrected with this:
http://hg.code.sf.net/p/octave/io/rev/4ec046f28e6d

Philip Nienhuis <philipnienhuis>
Group Member
Wed 29 Dec 2021 08:23:24 PM UTC, comment #59: 

Pushed toJSON.m and fromJSON.m (both 2.7.4) minus code update stanzas here:
http://hg.code.sf.net/p/octave/io/rev/5985cb78d718
http://hg.code.sf.net/p/octave/io/rev/e7a928b22fc2

Again, thanks very much for your perseverance and -last but not least- patience, Ketan!

Closing report.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 29 Dec 2021 03:24:06 PM UTC, comment #58: 

'numerical.json' was just to show that 'fromJSON', under proper circumstances, is no slouch on conversion speed, even for big files.  (obviously, no match for pending release of 'jsondecode').  In case you want to compare out of two decoders, element-by-element, I had included a smaller version, 'numerical_small.json', for easier content inspection. 

On texinfo, thanks for your edits (and double checking the function outputs).  I'm okay with your changes.  I did make style change to your edit in 'fromJSON'.

On the code comments, I modified them per your style guideline.

On the revision tracking, I'm sorry for clumsy attempt.  I started it to keep track for myself.  I'm not software dev and am not familiar with best practices.  I'm more than happy to defer to your judgement.

Thanks for your work in maintaining 'io'.

K

(file #52583, file #52584)

Anonymous
Tue 28 Dec 2021 03:34:35 PM UTC, comment #57: 

Comparing fromJSON_v2_7_3 & jsondecode:

>> fid = fopen ('numerical.json')';
>> js = fread (fid, Inf, "*char")';
>> fclose (fid);
>> tic; out1 = fromJSON_v2_7_3 (js); toc
Elapsed time is 5.74516 seconds.
>> tic; out2 = jsondecode (js); toc
Elapsed time is 0.167249 seconds.
>> whos
Variables visible from the current scope:

variables in scope: top scope

  Attr   Name        Size                     Bytes  Class
  ====   ====        ====                     =====  =====
         ans         1x1                          8  double
         fid         1x1                          8  double
         js          1x18787793            18787793  char
         out1        1x1                    8016000  struct
         out2        1x1                    8016000  struct

Total is 18787797 elements using 34819809 bytes


... so I infer fromJSON (v.2.7.3) and jsondecode produce the same output (didn't check  in detail; though a cursory inspection in the variable editor shows they aren't complicated data structures).

What I did next:
run both files through dos2unix to change CRLF EOLs into LF EOLs; adapted fromJSON's & toJSON's Texinfo a bit to better comply with Octave conventions (the very first sentence, not a paragraph, is merely meant to concisively describe what the function does) and esp. in fromJSON I've collected all info on SARRAY into the second input argument description. Also I fixed some scattered typos, and the results + diffs compared to your latest versions are attached.  In those diffs you'll be interested mostly in the Texinfo; I didn't touch the rest except for removing trailing whitespace.

What I still want to do is morph the info in the "Updates" section below the Texinfo into the commit message, that'll hopefully shorten the code while your work is still documented in the Mercurial log.
Of course, all of this if you agree :-)
One last remark: Would you mind if I change all comment line starts from '%' into '##' (or '#' when comments follow statements on the same line)? Both functions can't run in Matlab anyaway (if only for the texinfo header) and that way the io package code is more Octave style compliant  - an aim of me as package maintainer.

Whatever, I'm impressed with your work, Ketan, thank you very much.

(file #52576, file #52577, file #52578, file #52579)

Philip Nienhuis <philipnienhuis>
Group Member
Wed 22 Dec 2021 07:53:17 AM UTC, comment #56: 

I follow this discussion with some interest.  @K you can test jsondecode at any time in any Octave version using the json package:


pkg install "https://github.com/gnu-octave/pkg-json/archive/v1.5.0.tar.gz"
pkg load json


And make the experiments yourself:


>> str = fileread("numerical.json"); tic; fromJSON (str); toc, tic; jsondecode (str); toc
Elapsed time is 4.49402 seconds.
Elapsed time is 0.136716 seconds.
>> str = fileread("numerical_small.json"); tic; fromJSON (str); toc, tic; jsondecode (str); toc
Elapsed time is 0.00621009 seconds.
Elapsed time is 0.000169039 seconds.
>> str = fileread("dhs_keywords.json"); tic; fromJSON (str); toc, tic; jsondecode (str); toc
Elapsed time is 0.231829 seconds.
error: jsondecode: parse error at offset 1: Invalid value.


However, jsondecode is compiled C++ code, using the fastest free JSON C++ library available today.  Any m-code can hardly compete against this, but your code seems as good as possible using pure m-code 🙂

Kai Torben Ohlhus <siko1056>
Group Member
Wed 22 Dec 2021 05:33:09 AM UTC, comment #55: 

Got it. 

On your 'xulstore.json' trial: this was an issue cause by a URL being used as a struct key.  I found that in other files too and resolved earlier today.

Likewise on the stricter Octave syntax checking.  I was able to get Octave 6.4 running and saw the same.  This too has been resolved.   I assume it will likewise be OK in 7 and 8.

On the 'large-file.json', it's a good hard-case, but as noted before, this kind of JSON file is not the main use-case for (Octave or 'fromJSON') and will definitely choke on 'fromJSON' recursions. If, on the other hand, you were to try with a numerical dataset (in Octave's  wheelhouse), like 'numerical.json' (19MB) found here:

https://gist.github.com/kmpatel/e37e4df0a1971f25fcf10fadd2d8368d

'fromJSON' is much, much, much, much faster (~3sec on a 4th gen quad core).  Would be curious to see difference is speed and output with 'jsondecode'.

Also, regarding the early issue with Seamonkey profiles, I think I might have found a similar file (attached).

In the attached file, 'dhs_keywords.json', the 3 leading bytes are neither valid ASCII chars nor, AFAIK, a valid UTF-8 char.  As such, they do even display on screen (in Octave or any texteditor).  More over, since they are unquoted, they have no valid JSON related reason to be there and causes problems in 'fromJSON'.   I assume it is some file byte code for govt spying.

Anyways, on the assumption that THIS is the "Seamonkey profile" problem, I performed the necessary bug fix of simply detecting and ignoring any invalid UTF-8 at beginning of string (on the premise that these are file encoding char accidently read in by 'fread' or 'fileread').  Anywhere else in the file, if they are quoted, valid (like emoji's) and invalid UTF-8 chars will pass thru harmlessly, but unquoted invalid UTF-8 char will continue to be a problem.  'fromJSON' will fail gracefully with a <invalid frag> warning, showing the surrounding string, but the invalid UTF-8 chars will remain invisible to the user.  Unless they carefully inspect the string as a uint8 number array, as far as they can tell, the warning would appear bogus.

The attached 'fromJSON_v_2_7_3.m' has be tested against ~980 random JSON files found on my linux box. Quite a few were were not, strictly speaking, valid JSON files. Nevertheless, with the fixes to 'fromJSON' and a little pre-processing, these  JSON files now parses pretty well (I can account for those that don't).  I have incorporated some of these extreme conditions found (like your 'xulstore.json') in the BIST.  (At this point, there is 600 lines of BIST for 200 lines of code). 

I'm also uploading a improved Octave script I used to find JSON files, collect into a temp directory, and test against 'fromJSON'.  I think it will work on Win and Mac, if you want, but you will have to change the directory where all the JSONs are collected (or at least modify script to simply read in place).


K

(file #52551, file #52552, file #52553)

Anonymous
Tue 21 Dec 2021 10:04:21 PM UTC, comment #54: 

Yeah, parsing JSON is hard.

fromJSON-2.7.2 is much more stable, thanks, earlier version could crash on several JSON files I tried.

I tried xulstore.json from my Firefox profile dir, see jsontrials.mat (attached), where ans1 is from fromJSON and ans2 from jsondecode. To make it easy I attached a "readjson.m" function kludge to read a json file into a text string. One can see several differences. No idea if they're relevant.

Furthermore I tried another big JSOn file from the web, see
https://github.com/json-iterator/test-data/blob/master/large-file.json
On my laptop with Ryzen 7 5800U reading with fromJSON consumed 14 GB of 15 GB available RAM (pfew, no trashing) and ran for more than an hour before I gave up and went to bed. jsondecode did it within a few seconds. Yet to be fair, parsing this large JSON files (25 MB) with a .m-script-based parser is just far too ambitious. As for me I envision fromJSON and toJSON to be used on "reasonable" files and data structures.

Yet when reading that big file fromJSON emitted endless warnings along this line:

:
warning: colon arguments should be scalars
warning: called from
    fromJSON>_to_array_ at line 191 column 7
    fromJSON>_fromjson_ at line 127 column 22
    fromJSON>_to_struct_ at line 222 column 22
    fromJSON>_fromjson_ at line 128 column 22
    fromJSON>_to_struct_ at line 222 column 22
    fromJSON>_fromjson_ at line 128 column 22
    fromJSON>_to_array_ at line 200 column 29
    fromJSON>_fromjson_ at line 127 column 22
    fromJSON at line 117 column 10
    readjson at line 6 column 9


... which looks like a thing from Octave 7 & 8 where syntax checking became a little stricter. I tried to debug this but couldn't pinpoint where it came from. I saw no colons but they may be hidden in the eval command input. In the latter case it isn't so much fromJSON's fault per se but maybe it should be more robust.


(file #52545, file #52546, file #52547)

Philip Nienhuis <philipnienhuis>
Group Member
Mon 20 Dec 2021 09:17:57 PM UTC, comment #53: 

Per your goal, on testing 'fromJSON' on random JSON files, I collected all the JSON files on my linux sys with something akin:


mkdir(td="/tmp/test");
system(['find ~ -name "*.json" -exec cp {} "' td '" \;']);

lastwarn('');

for f=dir([td "/*.json"])';
  if( f.bytes < 1e6 );
    printf("\r%8d  %40s",f.bytes, f.name);
    fid=fopen([td '/' f.name]);
    fromJSON( fread(fid,'*char')' );
    lastwarn && printf("\n") && lastwarn('');
    fclose(fid);
  end
end;


This can detect 'fromJSON' parsing errors (via warnings), not necessarily mangled outputs.   Nor is this strictly valid, in that I came across some files that were not strictly valid JSON; e.g. some files had c-style comment (with an explicit warning to remove all non-JSON c-style comments 😂).

On that note, I did find another bug (where firefox uses an URL for a object key, UGH!), which is now fixed (to be uploaded later).

Testing for accuracy in 'toJSON' or 'fromJSON' is tough, for two reasons: whitespace and floating-point numbers precision.

With random JSON files, I tried doing something on the order of:

  strcmp(json, toJSON(fromJSON(json)))


but

  • some JSON string will be beautified with whitespace: extraneous spaces and newline for human use only (not for parsing).    'toJSON' doesn't beautify, nor can it replicated the limitless permutation that may been used in the original JSON string.  (and uglifying random JSON files is non-trivial).


  • some JSON have floating point number strings that push the limit of 64-bit FP precision.  Converting back and forth doesn't guarantee the EXACT same FP number string.


Let me look into how to do comparison using NodeJS. If I figure out something, I will upload a NodeJS script, along with the aforementioned bug fix.

Anonymous
Sat 18 Dec 2021 06:54:48 PM UTC, comment #52: 

Thanks Ketan for all the hard work.
I think I'll download some random JSON files from the web to try fromJSON's robustness. I have no idea how to test toJSON other than recoding fromJSON's results and feeding them to the program that produced or used them.

Probably around Xmas I'll start preparing for a new io release. But it'll be finalized only when you're confident that toJSON and fromJSON are sufficiently reliable.

Philip Nienhuis <philipnienhuis>
Group Member
Sat 18 Dec 2021 01:27:12 PM UTC, comment #51: 

some more tweaks (`fromJSON_v_2_7_2.m`).  o/(╥ᆺ╥)\o

K

(file #52528)

Anonymous
Sat 18 Dec 2021 05:38:21 AM UTC, comment #50: 

Sorry,  found another bug in "fromJSON".  Also fixed some typo and updated copyright date. 


Uploading 'toJSON_v_2_7_1.m' and 'fromJSON_v_2_7_1.m'

K

(file #52524, file #52525)

Anonymous
Thu 16 Dec 2021 06:37:03 PM UTC, comment #49: 

I have upload new 'fromJSON' and 'toJSON': "fromJSON_v2_7.m" and "toJSON_v2_7.m"

On both, I fixed bug, tweaked feature, refactored a little to clean and speed up the code, add more tests, and updated help text.

In the process of fixing the bug noted before, I realize that I had tunnel-vision in focusing on parsing numerical dataset.  As such, I made some minor feature change in how SARRAY effects parsing arrays.  So now 'fromJSON' can more safely parse JSON like those found in Seamonkey profile.  This is not to say it will replicate 'jsondecode' output, but rather 'fromJSON' will not mangle the output.

Additionally, for the naive user, I updated the HELP text to more clearly emphasize the preferred use-case for 'fromJSON' and 'toJSON', namely the focus on dealing with numerical arrays, Inf, and complex numbers which are neglected by JSON spec and "other" JSON parser.

Anonymous
Tue 14 Dec 2021 08:33:17 PM UTC, comment #48: 

Nice catch.   There is a bug. 

The one you encountered involves leading whitespaces in arrays.  I also found another bug involving the timestamp string inside an array being confused for a data structure.  

I fixed both issues, but haven't uploaded yet.  I'm working on developing some BIST tests involving these cases.  Please give me couple of days get some BIST in place.

The former is definitely bug and needs to be (has been) addressed before release of io.

The latter is a quasi-bug.  I did fix fromJSON for this specific case, but I can't be sure of other cases.  But this kind of problem was anticipated, for which I had include "SARRAY" (T/F) as an optional input.  Recall that 'toJSON'/'fromJSON' caters to transmitting large, numerical dataset thru JSON, including curing defect in JSON specification (i.e. JSON spec claims to be permit IEEE754 numbers, but doesn't actually include provisions for NaN, -Inf or Inf...UGH!!!!!), while 'jsonencode'/'jsondecode' does not (a grave oversight by Matlab and Octave teams, IMHO).

While it's correct to expect JSON string like those found in Seamonkey profile dir to be properly parsed, it is not the main use-case for 'fromJSON' (or Octave for that matter).   As such, 'fromJSON' defaults to parsing JSON as numerical arrays.  It is incumbent on the user to set "SARRAY" to FALSE for safer, slower parsing where confusing complex syntax (like that in Seamonkey profile JSON) might cause error.   I have amended the help doc to better emphasis this point.  I might update warning too.  I know you are unfamiliar with JSON usage, but when I upload, I hope you take a look at the wording and provide feedback on this.

K

Anonymous
Mon 13 Dec 2021 08:35:41 PM UTC, comment #47: 

I tried a few json files from Seamonkey's profile dir. I didn't see file encoding errros this time but saw a few differences between fromJSON and Octave's core jsondecode functions:

>> fid = fopen ('blocklist-gfx.json')
fid = 3
>> json = fread (fid, Inf, "*char")';
>> fclose (fid)
ans = 0
>> a = fromJSON (json);
warning: fromJSON.m: invalid frag: "
    {
      "vendor": "0x10de""
warning: fromJSON.m: invalid frag: ": "g200""
warning: fromJSON.m: invalid frag: ": []"
warning: fromJSON.m: invalid frag: ": true"
warning: fromJSON.m: invalid frag: ": {
        "who": "All Firefox users.""
warning: fromJSON.m: invalid frag: ": "Security problems.""
warning: fromJSON.m: invalid frag: ": "https://bugzilla.mozilla.org/show_bug.cgi?id=80955
0""
warning: fromJSON.m: invalid frag: ": "Mac OS X WebGL anti-aliasing""
warning: fromJSON.m: invalid frag: ": "2012-11-12T10:35:32Z"
      }"
warning: fromJSON.m: invalid frag: ": "ae36a32e-5f4f-2f98-804f-22b0de5e7bfa""
warning: fromJSON.m: invalid frag: ": "BLOCKED_DEVICE""
warning: fromJSON.m: invalid frag: ": "Darwin 11""
warning: fromJSON.m: invalid frag: ": 1480349135384"
warning: fromJSON.m: invalid frag: ": "WEBGL_MSAA""
warning: fromJSON.m: invalid frag: ": 1480349134676"
warning: fromJSON.m: invalid frag: ": "synced"
    }"


while core Octave jsondecode emits no warnings:

>> a = jsondecode (json);
>>


No idea if this is relevant at all. As I wrote before, I'm unfamiliar with JSON.
(file 'blockinglist-gfx.json' attached)

They're just warnings; I note different style outputs but we knew that fromJSON isn't Matlab-compatible and we agreed that it could be a Good Thing some comments ago didn't we.

Indeed I wrote that as package maintainer and non-JSON-user I have no need to improve fromJSON, I think that's entirely your pick as author.

So if you want to amend the regexprep warning code, here's your chance.

I'd like to make a new io release before New Years Eve, in fact this fromJSON thing is actually what the release is waiting for. I also wrote that fromJSON fixes could also wait for io-2.6.5, but seeing that current io-2.6.3 is over a year old now another year may be a bit long ...



(file #52501)

Philip Nienhuis <philipnienhuis>
Group Member
Fri 03 Dec 2021 01:02:29 PM UTC, comment #46: 

Thanks, I'll try your suggestion this weekend on the JSON files in my browser's profile subdir.
I wouldn't know how to provoke another regexprep error than encoding stuff so that I can test the "else" clause, but just let me think about it.

BTW I'm with you about the need for clear and useful error messages. But if an unwary user is put off by "regexprep" etc. I hope he/she will rush to the help & support forums here.

Philip Nienhuis <philipnienhuis>
Group Member
Wed 01 Dec 2021 11:26:15 PM UTC, comment #45: 

I recognize what you and Markus are saying about the potential of misleading message.  And I won't argue that point. Nevertheless, I'm hesitant to simply pass emit "regexprep" error as is.  While you and Markus and I would recognize the proper course of action, I'm thinking of the unskilled end user.  For him/her, seeing a error from "regexprep" could be confusing, if: 1) they don't know what/why "regexprep" is, 2) doesn't recognize that "regexprep" is choking on the input JSON string (as opposed to some other string, like the regex pattern string).

Regardless, where you suggesting to leave "fromJSON" alone for now?

Or alternative, I could implement a screen for this very specific, likely error, e.g.:


try
  str = regexprep(str, '^[\s,]*','');

catch err

  if( regexpi(err.message, "invalid(.*)UTF-8") );
    error("Invalid UTF-8 encoding of JSON string");
  else
    error(err);
  end

end_try_catch


Unfortunately, I'm unable (unskilled) to get Oct 7 built.  Thus, I'm still unable to replicate and test this against offending JSON strings.

K

Anonymous
Mon 29 Nov 2021 10:06:48 PM UTC, comment #44: 

Markus (comment #38) merely meant to say that the reason mentioned in the error error message ("invalid UTF-8" )in the try-catch block COULD be incorrect.
He's absolutely right there and I agree. I also think regexprep is sufficiently "grown-up" to emit a proper error message by itself, no try-catch needed.
Yet I also wonder how bad it could be in practice and whether io-2.6.4 release should wait for it to be adapted.
fromJSON and toJSON + the entire io-2.6.3 package have been released over a year ago and we've seen no complaints yet.

I would be fine with leaving things as-is for the time being. After all, some time after io-2.6.4 there'll be an io-2.6.5 or maybe 2.8.0 I suppose :-)
==> no worries

Philip Nienhuis <philipnienhuis>
Group Member
Mon 29 Nov 2021 06:37:23 PM UTC, comment #43: 

Hi

I'm reviewing my notes.

I would be happy to fix the problem, but as I noted before, I'm unable to replicate the bug.  As far as I can tell, it doesn't seem to be a problem in current release (6.4).  If it is, then I really need help replicating error.  I've tried number of different methods to import and parse non-UTF-8 encoded JSON strings but couldn't generate an error from "fromJSON". 

Anyways, regarding encoding issue specifically, I'm hesitant to let regexp error propagate (i.e. drop the try-catch).  While I agree that the meaning of the error is clear, it allows the end-user to presume that the solution is found in modifying "fromJSON" rather than dealing with input string. 

If your concern about the exact wording of error, I'm fine with modifying it in line with your suggestion. 

On the issue of pointing to the offending char, I agree it would be nice to have, but you rightly point that this is a regexp-issue and as such beyond the scope of "fromJSON".


Give me a couple more days to try to get Oct 7 built and try to replicate error myself.

K

Anonymous
Fri 26 Nov 2021 11:30:18 PM UTC, comment #42: 

(reopening temporarily)

I'm about to prepare an io-2.6.4 release so was looking at this item in comments #38-40 that is still on my "io" to-do list.

What's the advice? just drop the try-catch block cf. comment #38? or change the error message into s/th along the lines of:

error ("Maybe invalid UTF-8 encoding of JSON string ?");


Maybe even throw lasterr before the error message? (actually equivalent to dropping the try-catch)

I'm with Markus in that it's probably better to just let regexprep show its error message followed by the stack trace. But for the situation at hand, is regexprep's error message clear enough when it crashes due to encoding issues? I just don't know.

On another note, when hitting errors while parsing strings I find it often handy if the location of the offending characters in the string is returned. But that may be asking too much here :-) (I suppose that would be regexprep's job)

(sattus => need info)

Philip Nienhuis <philipnienhuis>
Group Member
Sat 07 Nov 2020 09:17:27 PM UTC, comment #41: 

I guess this is for io-2.6.4 then (with 2.6.3 just released).

Philip Nienhuis <philipnienhuis>
Group Member
Sat 07 Nov 2020 05:22:14 PM UTC, comment #40: 

Usually Octave displays errors with their stack. That should make clear that regexprep was called from `fromJSON` in this case.

Markus Mützel <mmuetzel>
Group administrator
Sat 07 Nov 2020 01:17:54 PM UTC, comment #39: 

comment #38:

> Sorry, I didn't read the changeset earlier. But this snippet might give misleading error messages:
>
> It might be better to remove the try-catch-block and let regexprep display an error message directly.


You are correct that the cause of the error could be misleading and it bothered me too.  But I'm not sure that resorting to 'regexprep' error is a better solutions.  

The problem I wanted to avoid is if 'fromJSON' was buried in another script/function, an error from 'regexprep' (rather than 'fromJSON') would be confusing or misleading too.  I know it is always possible to trace the problem to 'fromJSON' but not easily for novices.

Initially I tried to post-process the error from 'regexprep', but I could not replicate the error and verify that the solution was adequate.  As such, I opt with this.

While I too am unsure about what else might cause 'regexprep' error, since 'str' is already screen for classtype, unless 'regexprep' changes methods, I think it could be safe to say that a runtime error would only be from 'str' content.  Maybe not UTF-8 specifically but content nonetheless.

At best, it might be "safer" to just return a generic, 'Invalid JSON string' and drop the reference to UTF-8.  But I'm on the fence as to whether that is any better.

Or if someone share with me a sample file or method to replicate the problem, I can fix this better.

I await suggsetions....

Anonymous
Sat 07 Nov 2020 08:56:38 AM UTC, comment #38: 

Sorry, I didn't read the changeset earlier. But this snippet might give misleading error messages:

+  try
+    str = regexprep(str,'^[\s,]*','');
+  catch
+    error("Invalid UTF-8 encoding of JSON string");
+  end_try_catch


I am not certain if the only reason for which regexprep can possibly fail is invalid UTF-8. If it fails for a different reason, the error message that is shown might lead in the wrong direction.

It might be better to remove the try-catch-block and let regexprep display an error message directly.

Markus Mützel <mmuetzel>
Group administrator
Mon 02 Nov 2020 06:33:49 PM UTC, comment #37: 

Thanks

Anonymous
Sun 01 Nov 2020 06:22:12 PM UTC, comment #36: 

I pushed the functions here:
http://hg.code.sf.net/p/octave/io/rev/6bb1f2a0e622
after a miniscule edit (add a newline at the end of one of the functions).

(Hmm I should add them to the INDEX file as well, will do.)

Thanks very much Ketan for your contributions here and the prompt replies.

Closing report as "Done".


Philip Nienhuis <philipnienhuis>
Group Member
Sun 01 Nov 2020 05:49:13 PM UTC, comment #35: 

The texinfo changes are good.  I tweaked a little on the wordage.

Likewise on pre-pending warning msg.

I understand the comment on the caution, it occurred to me too, but since the ND array outputs might appear okay, I worried end-user (current Matlab or future Octave 7) will not notice the reshaping.  Nevertheless, I'm OK with dropping it.

On whitespace, I added some more, at least on func def.  On func calls, I prioritized keeping code width under 80chars (and uniform) over more whitespace.

On single/double quotes, I sought to use single-quote on all symbols (single or multi-char) and double-quote on all sentence frags (i.e. with whitespaces).  In some places, however, single- and double- quote are not interchangeable, due to need of \ or " literals.

(file #50183, file #50184)

Anonymous
Sun 01 Nov 2020 02:06:39 PM UTC, comment #34: 

OK.
Note that your email is still in the functions themselves.

I've uploaded them here with the following changes:

  • Fixed & reorganized texinfo so that info is somewhat more organized per subject / input arg. I hope you like it.  Fixing Texinfo issues often boils down to adding "@" before offending characters ("@" is a Texinfo control char) and I use the on-line Texinfo docs:

https://www.gnu.org/software/texinfo/manual/texinfo/texinfo.html

  • Dropped caution at end of texinfo. I think we shouldn't refer to functions in future Octave releases (jsonencode/jsondecode are in 7.x which will be released maybe only later next year) or the proprietary competition in any way.
  • Updated copyright strings
  • Prepended warning- / error messages with function name.


and apart from minor typos and Octave's editor removing whitespace after each line, that's it.
For convenience I've also uploaded "diff -u" diffs to see what got changed.

There are still smaller style things (spaces between function arguments/after comma's, between function name and opening parenthesis, double rather than single quotes where possible) but I'm not going to chase that all down.
As I wrote in my first comment here: on the whole I think you've done a very good job.

If you want to make some last edits, here's your chance. If (when) you are satisfied I'll push them, just let me know.


(file #50178, file #50179, file #50180, file #50181)

Philip Nienhuis <philipnienhuis>
Group Member
Sat 31 Oct 2020 11:02:21 PM UTC, comment #33: 

Okay on name, but not on email.  I'm okay with it in file but not on searchable website.  You can thank Google for that.

Anonymous
Sat 31 Oct 2020 05:05:59 PM UTC, comment #32: 

No need to apologize :-)

If you think the latest function versions are OK I'll test them briefly, push, and then polish Texinfo adapt style.

Would you mind if I push them with your name and you're credited in the commit message? If so can I use your name and email? (your email isn't mentioned in the on-line log, only when browsing he actual commits)
I'm asking because there are contributors who explicitly want to remain anonymous.


Philip Nienhuis <philipnienhuis>
Group Member
Sat 31 Oct 2020 03:41:13 PM UTC, comment #31: 

very very sorry.

uploaded the wrong file!!!  do not use "fromJSON_v2_3.m"

use "fromJSON_v2_4.m"

(file #50177)

Anonymous
Sat 31 Oct 2020 03:36:21 PM UTC, comment #30: 

sorry, I didn't do it 'native2...' correctly. 

now its an error for improperly encoded string. please use "fromJSON_v2_3.m"

(file #50176)

Anonymous
Sat 31 Oct 2020 03:35:22 PM UTC, comment #29: 

Sorry. I probably wasn't clear. `fromJSON` should not call `unicode2native`. Instead, the user (in the case of comment #21: Philip) must make sure that they call the function with valid strings.
Imho, it is ok if an error is thrown if the function is called with invalid strings.

Markus Mützel <mmuetzel>
Group administrator
Sat 31 Oct 2020 03:28:00 PM UTC, comment #28: 

comment #26:

> The "invalid UTF-8" error is probably because `regexp` requires UTF-8 encoded input in Octave 6 or newer.
> I guess the input file has a different encoding. In Octave 6 or newer you can specify the encoding when opening a file with `fopen`. Otherwise you could use `native2unicode` to convert from whatever input encoding to UTF-8.


I've taken this advice and fixed the input with 'native2unicode' and a warning.  I can only verify that code did not break, I'm still at a loss to generate and test against a wrongly encoded string.   In Octave 7, I've tried writing out and reading back in a JSON string with non-UTF-8 encoding (including bizzare characters).  I was unable to replicate the `regexp` error (on Oct 5 or 7), so I can't tell if this fix works.

On the issue of multidim arrays, I think "to/fromJSON" and "jsonencode/decode" will have to go separate ways. I've updated the HELP doc to reflect this.  IMHO, Matlab's choice is at best inconsistent and at worst just wrong. Maybe "to/fromJSON" is wrong too, but I think at least its more consistent.   so...

Files are uploaded ("to/fromJSON_v2_2.m").  I have included some of the big BIST tests from "jsonencode" and "jsondecode".


(file #50174, file #50175)

Anonymous
Fri 30 Oct 2020 04:26:50 PM UTC, comment #27: 


>This brings me to a question.  At one time, I had the following BIST:
>    %! r = round(1000*rand(10,10,10,10,10));
>    %! assert(r, fromJSON( toJSON(r)));
>but later took it out to avoid referring to an external function.  >Can I put it back in?

If with "external function" you mean "round" and/or "rand", that seem OK to me. In core Octave many many BIST tests rely on behavior of other core functions.
Same for calling "toJSON" in a BIST for "fromJSON". We want to test  functions in a real environment where they are all available, isn't it?
Or do I misunderstand your question?

Philip Nienhuis <philipnienhuis>
Group Member
Thu 29 Oct 2020 04:38:51 PM UTC, comment #26: 

The "invalid UTF-8" error is probably because `regexp` requires UTF-8 encoded input in Octave 6 or newer.
I guess the input file has a different encoding. In Octave 6 or newer you can specify the encoding when opening a file with `fopen`. Otherwise you could use `native2unicode` to convert from whatever input encoding to UTF-8.
AFAICT, this is not a problem of `fromJSON` (which accepts strings). The problem is that an invalid string is passed to that function. The file has to be read with the correct encoding or the read byte stream has to be converted from that encoding to a valid string.

Markus Mützel <mmuetzel>
Group administrator
Thu 29 Oct 2020 04:34:25 PM UTC, comment #25: 

I've added the "warning('off'...".  Turns out some of those warnings were inadvertent errors the BIST.

On the "Invalid UTF-8" issue, I'm having trouble finding/generating an offending JSON string.  Can you show rest of code on how it was read into Octave?  And/or upload a sample file? 

Anonymous
Thu 29 Oct 2020 03:19:22 PM UTC, comment #24: 

On comment #21, my code was wrong to allow the '\"' pass thru as is, I fixed that.   Turn's out my BIST for that check was wrong too.   I will upload soon.

On the changing of "remote-settings/monitor_changes" to "remote-settings_monitor_changes" (along with other fixings), I agree with you and not that design choice.  Provided it doesn't cause "fromJSON" to exit in error, I'm okay with "garbage in-garbage out" and leave it to the end user to catch and fix as he/she sees fit.  The "invalid UTF-8" error, on the other hand, bothers me.  Let me see what I can do.

On comment #19/20 vis a vis the ND array:

the difference is not a bug but difference in interpretation.  I  look (with my own eyeballs) at the JSON string and see multiple pages of 2x2 matricies, and "fromJSON" reflects that interpretation.  But it would seem jsondecode sees it as pages of 2x4 matricies.

You can see the difference more clearly in a simpler example (hypothetically-generated via "fromJSON_BIST.tst", you should check that manually):

jsonA = '[[1, 2], [3, 4]]';
mat1A = jsondecode(jsonA);
mat2A = fromJSON(jsonA);

jsonB = '[[[1, 2], [3, 4]], [[5, 6], [7, 8]]]';
mat1B = jsondecode(jsonB);
mat2B = fromJSON(jsonB);


returns the following.  One of these things is not like the others.


mat1A(:,:,1) => [1 2;3 4]    ## <-- inferred from "fromJSON_BIST.tst"
mat2A(:,:,1) => [1 2;3 4]    ## <-- actual

mat1B(:,:,1) => [1 3;5 7]    ## <-- inferred from "fromJSON_BIST.tst"
mat2B(:,:,1) => [1 2;3 4]    ## <-- actual


Which is correct: 'mat1B' or 'mat2B'? Actually, I don't know if this is really a difference in interpretation or ND arrays hurts the brain of the original "jsondecoder" developer too.  Either way, I prefer the output of "fromJSON", a more intuitive flow from "jsonA" to "jsonB".    BTW, "object2json" sees it the same as "jsonencode/jsondecode" and avoiding this was deliberate effort I made.

This brings me to a question.  At one time, I had the following BIST:


%! r = round(1000*rand(10,10,10,10,10));
%! assert(r, fromJSON( toJSON(r)));


but later took it out to avoid referring to an external function.  Can I put it back in?

Again, I feel a JSON parser should try to work to mate Javascript to Octave/Matlab, and Javascript only knows c-style, nested, 1-D arrays not FORTRAN-style multidim arrays.    In this regards, "fromJSON" and "toJSON" do not respect Octave column vectors: they treat any vector as a 1-D Javascript row vector...ALWAYS.

Anonymous
Wed 28 Oct 2020 05:21:39 PM UTC, comment #23: 

NaN vs. [] is probably a matter of Matlab-compatibility, which is of more importance for core Octave than for O-F packages.
In this case all the more so as your functions are named differently than in Matlab to start with.
So, with that out of the way I agree with you on this detail.

As to complexity of ND-arrays, I'm with you too :-)

I think it's worthwile to check out the tests I uploaded in comment #20 and check if there are no alarming differences or bugs.
AFAICS most of the tests work in toJSON and fromJSON.


Philip Nienhuis <philipnienhuis>
Group Member
Wed 28 Oct 2020 03:53:50 PM UTC, comment #22: 

On the quoted quoted-string, that's could be a bit tricky.  I will look into it.

On the difference in '[]' and 'NaN' for "null"...originally "fromJSON" conditionally return '[]' for "null": 'NaN' if in numerical array, '[]' if in mixed-class array.  However, given that Octave is a numerical computing environment, I made a deliberate choice to instead always make "null" into 'NaN'.  Aside form the symmetry to "toJSON" reflexively turns all 'NaN' to "null", "fromJSON" does recursive processing, and I worried about some edge-cases causing confusion.

I attribute this comprimise to what I regard as gross (teeth-grinding!) negligence in the JSON spec, which fails to accommodate for 'NaN' and 'Inf' despite both being valid IEEE754 values and valid Javascript values.  Besides, an empty-array is possible in JSON, so converting "null" to '[]' is a similarly dubious comprise.

In the end, with regards to NaN, [], UTF encoding, etc., this comes down to what is being transported via JSON: numerical dataset or information.  I was setting out to solve the problem of transporting numerical data (i.e. numerical ins and outs of Octave functions as backend to Javascript), where as 'jsonecode/decode' might have settled on transporting information (i.e. processing HTTP requests, etc).

On the ND array, I will look at that.  ND arrays hurts my brain, so I still have to wrap my head around what you show.

Anonymous
Wed 28 Oct 2020 08:42:37 AM UTC, comment #21: 

Just curious and for some more evaluation, in Octave-7.0.0 I tried fromJSON and jsonencode on some random json file in my Firefox and Seamonkey profiles (lots of json files there),
For example:

>> file = 'broadcast-listeners.json'
file = broadcast-listeners.json
>> fid = fopen (file);
txt = fread (fid, Inf, "*char")';
fclose (fid);
>> txt
txt = {"version":1,"listeners":{"remote-settings/monitor_changes":{"version":"\"1603569466778\"","sourceInfo":{"moduleURI":"resource://services-settings/remote-settings.js","symbolName":"remoteSettingsBroadcastHandler"}}}}

>> fromJSON (txt)
ans =
  scalar structure containing the fields:
    version = 1
    listeners =
      scalar structure containing the fields:
        remote-settings/monitor_changes =
          scalar structure containing the fields:
            version: 1x17 sq_string
            sourceInfo: 1x1 scalar struct
>> ans.listeners.('remote-settings/monitor_changes').version
ans = \"1603569466778\"

>> jsondecode (txt)
ans =
  scalar structure containing the fields:
    version = 1
    listeners =
      scalar structure containing the fields:
        remote_settings_monitor_changes =
          scalar structure containing the fields:
            version: 1x15 sq_string
            sourceInfo: 1x1 scalar struct
>> ans.listeners.remote_settings_monitor_changes.version
ans = "1603569466778"


Clearly Octave-7.0.0's jsondecode adapts structfield names and maybe struct field contents, I suppose for easier handling and maybe Matlab compatibility, while fromJSON delivers them 1:1. Yet fromJSON's output string with the escaped backslashes looks suspicious.
Again I have no idea if these results are right or wrong; opinions, anyone?

I'm inclined to think that it's a good thing that users can (also) get "raw" data from JSON files (provided they're correct). But that's me, being used to digging around in data files I tend to prefer to stay as close as possible to raw data before processing them my own way.

In another json file in my seamonkey profile I got

>> file = 'sessionstore-4.json';
:
>> jsondecode (txt);
## no errors
>> fromJSON (txt);
error: regexprep: the input string is invalid UTF-8
error: called from
    fromJSON at line 61 column 7


... but I suppose that's because the URL's inside can have varying encodings (peeking in the file I saw a lot of gunk).
Octave-7.0.0's jsondecode seems more resilient.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 27 Oct 2020 10:36:06 PM UTC, comment #20: 

(attachments gone after "Preview", sigh)
Here they are:

(file #50144, file #50145)

Philip Nienhuis <philipnienhuis>
Group Member
Tue 27 Oct 2020 10:34:35 PM UTC, comment #19: 

Thanks Ketan for the prompt updates.

With Octave-7.0.0:

>> test fromJSON
warning: Unclosed block: "[1,2,3"
warning: Unclosed block: "[[1,2,3],[3"
warning: Unclosed block: "[3"
warning: Unclosed block: "{a:3,b"
warning: Malformed struct frag: "{a:3,b ...}"
warning: Unclosed block: "{a:[[[1,3],[2,4]],[["
warning: Unclosed block: "{re:2,im:4"
warning: Invalid frag: "im mi{}ing quotes"
warning: Invalid frag: "garbage"
warning: Invalid frag: "garbage"
warning: Invalid frag: "garbage"
PASSES 68 out of 68 tests

>> test toJSON
PASSES 53 out of 53 tests


That noise can (and IMO should) be shut off; I did that by adding
%! warning ("off", "all", "local");
statements before the fromJSON calls provoking these warnings.

I've reworked the functional BIST tests for Octave-7.0.0's jsonencode and jsondecode and after some editing attached them here as toJSON_BIST.tst and fromJSON_BIST.tst.
I randomly chose the third "%% try higher dimensions for the array" test block in fromJSON_BIST.tst,


%! json = ['[[[[1,-1], [2,-2]],[[3,-3],[4,-4]]],[[[5,-5],[6,-6]],[[7,-7],', ...
%!         '[8,-8]]],[[[9,-9], [10,-10]],[[11,-11],[12,-12]]],', ...
%!         '[[[13,-13],[14,-14]],[[15,-15],[16,-16]]]]'];
%! var1 = cat (3, [1, 3; 5, 7; 9, 11; 13, 15], [2, 4; 6, 8; 10, 12; 14, 16]);
%! var2 = cat (3, [-1, -3; -5, -7; -9, -11; -13, -15], ...
%!             [-2, -4; -6, -8; -10, -12; -14, -16]);
%! exp  = cat (4, var1, var2);
%! obs  = fromjSON (json);
%! assert (isequal (obs, exp));


and saw some different dimensions wih your fromJSON.
Similarly for some toJSON_BIST.tst tests.
In other (earlier) tests I saw that where jsondecode returns [] for 'null', fromJSON returns NaN.
I hope you are willing to have a look at the tests in these files  and try them with the toJSON and fromJSON functions.

As I know nothing about JSON (well, I know it exists); can you inform me if the different test results are something to worry about?
Background:
I do not expect full Matlab compatibility. It's just that as package maintainer I'd like functions to produce correct output, that is, output sufficiently correct to be used out in the wild w/o provoking undue bug reports. After all, we hope these new functions will attract new users rather than put them off :-)
So do you think fromJSON.m and toJSON.m as they stand are "safe" enough to include in the io package, or is more work needed?

They can always be included with the word "EXPERIMENTAL" (in capitals) in the NEWS file.
That word is FWIW anyway :-)

On another note: once finally accepted,

  • Before pushing to the repo I'll do some style edits (notably single quotes => double quotes). I don't mind, but you may also do it;
  • I'll have a look at the Texinfo, no worries. I'm no Texinfo guru but I think I can improve it a bit here and there.


Lastly:
Apologies if you expected your contributions to go rapidly and w/o much edits into some Octave add-on package :-)  But correctness of functions is a Big Thing for me and the io package is a "community package", a category for which we think this counts a bit more than for external packages.
These functions will only get better this way.

Philip Nienhuis <philipnienhuis>
Group Member
Tue 27 Oct 2020 03:03:12 PM UTC, comment #18: 

Yes, the change was intentional and was made to improve compatibility with Matlab, which allows simple function handles to be created that refer to functions that are not currently defined in the search path.

John W. Eaton <jwe>
Group administrator
Tue 27 Oct 2020 01:30:13 PM UTC, comment #17: 

comment #15:

> What do you mean by
>


> v7 "break" of 'func2str'


>


By "break" I meant air-quoted "break". 

Its just that in v5.2, calling 'str2func("@nofunc")' where "nofunc" is non-existent returned an error, while in v7, it no longer is the case.  The resulting function will fail, of course, but 'str2func' will not.

I assume it is deliberate, but the change affected the behavior of my function.

Anonymous
Tue 27 Oct 2020 01:25:15 PM UTC, comment #16: 

Update 'toJSON' and 'fromJSON' have been uploaded. 

There were couple of other minor fixes needed for Octave 7 that produced no change in behavior, except one...

Octave 7 has changed its behavior regarding 'str2func'.  It is not as strict as in v5.2.  As a result, in v5.2, "fromJSON('@nofunc')" will return a string or function '@nofunc' depending on existence of 'nofunc' as a function; while in v7, "fromJSON" will always return a function '@nofunc', whether it is valid or not.

This is an edge-case scenario, in that transmitting functions over JSON is unusual in itself.  As such, to resolve this issue, I've simply removed the BIST test specifically for non-existing functions. 

Also, please note, the HELP doc remains broken with regards to layout.  I'm relying on some help on this issue.

(file #50136, file #50137)

Anonymous
Tue 27 Oct 2020 01:13:08 PM UTC, comment #15: 

What do you mean by


v7 "break" of 'func2str'


John W. Eaton <jwe>
Group administrator
Tue 27 Oct 2020 12:41:58 PM UTC, comment #14: 

I got Octave 7 going.

Regarding empty index, there was no issue with replacing '{}' with '{:}'.

Regarding issue with anonymous function, yes, I discovered that too.  Fix implemented with solution I had tried to originally avoid by using nested subfunction.

Will upload soon, as soon as I deal with v7 "break" of 'func2str'.  In this case, v7 has become LESS strict that v5.2.

Anonymous
Tue 27 Oct 2020 11:57:02 AM UTC, comment #13: 

The empty indexing error is Octave-7.0.0 AFAIK.
Same probably for the end indexing.

But

error: '_num2str_' undefined near line 170, column 170"


occurred in octave-6.0.92 as well; that'll be the new release shortly, so also (better) try that one first.
What I saw during a brief debug session is that anonymous functions in the callers are NOT in scope in subfunctions (callees), nested or not. From what I read in the docs their scoping mimics that of plain variables. So maybe you'd need to add them as subfunctions; or add them in the input args for called functions.

All of which makes me wonder how these functions could work at all in your case. Maybe Octave-5.2.0 was less strict?

I'm no fan of nested subfunctions anyway, usually one sooner or later gets lost in all kinds of scoping issues, like here. But this is all your pick :-)

Octave prerelease source tarballs and Windows prerelease installers are on ftp://alpha.gnu.org/pub/octave

Philip Nienhuis <philipnienhuis>
Group Member
Tue 27 Oct 2020 02:30:06 AM UTC, comment #12: 

Most information you will find in the wiki:

https://wiki.octave.org/Building

If you have trouble building Octave, please address a wider audience at https://octave.discourse.group/

Kai Torben Ohlhus <siko1056>
Group Member
Mon 26 Oct 2020 11:41:07 PM UTC, comment #11: 

BTW, where can I get Octave 7.0 source code?

Anonymous
Mon 26 Oct 2020 11:34:05 PM UTC, comment #10: 

Thanks.  I will look into ASAP.  I will try getting Octave 7.0 running first.

Anonymous
Mon 26 Oct 2020 08:07:21 AM UTC, comment #9: 

Thanks for the new patches.


Where in comment #2 I wrote that in Octave-7.0.0 these functions will become obsolete, if they are in the io package they should still run w/o issues for a few years until usage of then older Octave Out There is low enough. I think that is not unreasonable.

I checked and tested a few thing and I see the following in Octave-7.0.0:

>> test toJSON
***** test  ##  PREC input arg test
  assert(toJSON(pi,  0),'3')
  assert(toJSON(pi,  5),'3.1416')
  assert(toJSON(pi, -5),'3.1416')
  assert(toJSON(pi, 25),'3.141592653589793115997963')
  assert(toJSON(pi,  :),'3.14159265358979')
  assert(toJSON(pi, []),'3.14159265358979')
  assert(toJSON(pi, false),'3.14159265358979');  ## this is provoking stmt
  assert(toJSON(pi, {}),'3.14159265358979');          ## issue warning here
!!!!! test failed
invalid empty index expression


due to this statement on L.72:

    [PREC,COMPACT] = {[],PREC}{};


which (as we have discussed) is difficult to disentangle.
As of Octave-7.0.0 empty indexing e.g., var{}, is no longer allowed, see https://savannah.gnu.org/bugs/?56167.
Replacing by

%    [PREC,COMPACT] = {[],PREC}{};
    PREC = [];
    COMPACT = PREC(:);

makes it work but I don't know if that's the right fix.
The next test, 'toJSON(false)' fails elsewhere (L.234) and toJSON(reshape(1:8,2,2,2)) fails in L.217. Trying to fix those (just entering ":" between curly brackets) gave

"error: '_num2str_' undefined near line 170, column 170"


(I know it's defined on L.86) and then I gave up.
Please fix these issues.

Then,

>> test fromJSON
***** test  ## number
 assert(fromJSON('4'),4)
!!!!! test failed
invalid use of 'end': may only be used to index existing value
>> fromJSON('4')
error: invalid use of 'end': may only be used to index existing value
error: called from
    fromJSON>_fromjson_ at line 70 column 25
    fromJSON at line 62 column 9


(perhaps also an Octave-7.0.0 issue)
variable 'obj' needs to be initialized (empty) before it can be indexed with (end+1).

Also in fromJSON I saw several empty var{} indexing which will error out.

Can you build Octave-7.0.0 to try yourself?
If you're on Linux it shouldn't be too hard, if on Windows I could cross-build a recent tip for you and upload it somewhere.

I'm on the verge of making a (bug fix) io-2.6.3 release, I really hope you can delivr toJSON and fromJSON also running in Octave-7+ so that I can include them in the io package. In fact that's all the release is waiting for.
I hope you can fix all of these issues

Philip Nienhuis <philipnienhuis>
Group Member
Sat 24 Oct 2020 03:54:58 PM UTC, comment #8: 

I've uploaded a new version.  Aside from adding suggested testing improvements, it they worker harder at mating native JSON/Javascript objects with native Octave objects, namely ND arrays and struct arrays.

fromJSON
========

Per comments, some more input checking and BIST has been added.  Pointing to exact character position could be VERY helpful, but 'fromJSON' works recursively, start with big blocks of string and working down to smaller fragments.  As such, error position is a "nice to have".  Instead, 'fromJSON' cites relevant fragments when malformed JSON is found. 

toJSON
======

Per comments, more input checking and BIST have been added/fixed.  On exotics like java or COM, the norm for stringifying non-(number,bool,string) is a placeholder with a quoted-string of its class name (e.g. '"[SomeClass]"'), which 'toJSON' observes. Provided that 'class()' works on any Octave object, there are no "invalid" inputs, just unfullfilling outputs.  I've added a BIST test with java object for verification.

On other input tests, i.e. PREC and COMPACT (formerly JSON), done.

The input checking statement


isbool(PREC) && ([PREC,JSON] = {[],PREC}{});


is my preferred contraction for:


if isbool(PREC);
    JSON = PREC;
    PREC = [];
endif


The resulting bool value in irrelavent.  I understand comment about being complicated.

Regarding compatibility, 'toJSON' and 'fromJSON' is made to work with Javscript's primative objects and its built-in parser ('JSON.parse' and 'JSON.stringify').  With all due respect, that is the standard to which any JSON parser should rise.  If 'jsondecode' plays nicely with Javascript, then 'toJSON' should work with 'jsondecode'.

(file #50087, file #50088)

Anonymous
Mon 19 Oct 2020 12:00:48 PM UTC, comment #7: 

ok. 

had epiphany when discussing pro/cons against 'jsonlab', 'object2json' and 'jsoneconde'.

will address cited issues, and upload soon

Anonymous
Sat 17 Oct 2020 09:04:58 PM UTC, comment #6: 

@Ketan:

Input validation:

fromJSON:
=========

  1. checking that there is input at all (nargin >= 1)
  2. checking that input is a character string
  3. ditto that is has sufficient minimum length (how'd you parse ""? or is that accepted and if yes, what class output would I get, a double? char?)


If parsing fails, can fromJSON indicate at what character position it lost track? (is this useful? is it a nice to have? - I'm unfamiliar with JSON so I'm not afraid to ask stupid questions)

BIST for input validation look like this:
%!error fromJSON (true) <here your error message between angular brackets>
I core Octave you can find many .m-file functions with examples.

toJSON:
=======

  1. does it accept classdef objects? Java objects? COM objects? empty objects?
  2. what if PREC is a struct?
  3. same for JSON?


What does the code at L.63 actually do?

isbool(PREC) && ([PREC,JSON] = {[],PREC}{})


Assigment happens to [PREC,JSON] but the rest? IMO this is unneededly convoluted code. I tried several values and classes for PREC and JSON and consistently got 0 (obviously) that is assigned to nothing.

At the bottom of both functions there's "scraps" that don't belong there. Should either be before the tests or deleted.

Version tracking:
"help version"
"help compare_versions"

Does toJSoN produce "sufficiently compatible" output for "regular" input, that can be read by e.g., jsondecode? (or any other JSON parser Out There? Matlab? we aim at Matlab compatbility)
I've read and understand your musings in comment #3 so please a short answer :-)

Philip Nienhuis <philipnienhuis>
Group Member
Sat 17 Oct 2020 12:45:25 PM UTC, comment #5: 

Also, I looked into 'jsonlab' a little more. 

- it looks like its designed to a specific data format 'JData',  developed by the same author. https://github.com/OpenJData/jdata/blob/master/JData_specification.md

- it's designed with a specific, matching parser in mind; one for python, none for Javascript. Frankly, Octave-to-python seems moot because of NumPy.  Javascript on the other hand... https://github.com/fangq/jsonlab#sharing-jsonlab-created-data-files-in-python

> In short, to conveniently read/write data files created by
> JSONLab into Python, whether they are JSON based or binary
> JData/UBJSON based, one just need to download the below two
> light-weight python modules


- like 'object2json', 'jsonlab' can be slow.  speed, for some reason, is affected by ND array shape, not just size.  for example, on my desktop, "savejson(rand(1000000,1))" takes 1sec but takes 500sec for either "savejson(rand(1,1000000))"  or "savejson(rand(100,100,100))".  In contrast, "toJSON" takes 1sec for all 3 shapes and maintains the respective shape within the JSON file for straightforward parsing in Javascript and python (and others).  

- "loadjson" while appearing to work with non-JData style JSON files, mangles ND arrays similar to 'object2json'.  It assumes a strict FORTRAN style indexing not used in Javascript or python (plain), which use the c-style array indexing.  To be fair, "loadjson" is much faster than "fromJSON", presumably due to it's use of metadata.


Anonymous
Sat 17 Oct 2020 02:49:29 AM UTC, comment #4: 

comment #3:

> When I make the requested change, do I upload it here again?  how do I name or version number the new files?


Don't be too worried about this, just name it "toJSONv2.m" or alike.  It is not big deal to remove the "v2" suffix before testing it 😉

Kai Torben Ohlhus <siko1056>
Group Member
Sat 17 Oct 2020 01:28:31 AM UTC, comment #3: 

On input validation, Ok.  But "toJSON.m" is intended to take any and ALL objects and convert to JSON string, so I'm not sure what you have in minded there.  For "fromJSON.m", it works only on string, so that is no prob!   As for malformed JSON string, not sure how to valid other than just parsing the string.

Also, I don't know what you mean "yet input validation BIST tests are lacking"...as compared to what is already there.  The testing includes all the different Octave object classes I know off. More elaboration would be instructive.

On Octave version support, I don't know how to test for that, other than loading all versions on my computer.  The first iteration was develop on 4 but refactored on 5.  In refactoring, the only new feature added was default function args, so I'm 99.9999% sure it will still work on 4.  It uses strcut2cell, str2func, is_function_handle, cell2mat, regexp, regexprep and subsref/subsasgn, among other more mundane functions.

On JSON expertise, I don't know any.  I usually consult a web-browser's developer console (see JSON.parse).

On 'jsonlab', as well as the 'jsonencode' and 'jsondecode', I think 'toJSON' and 'fromJSON' still has some advantages.   I have very limited (<2hrs) experience with these so I cannot say expertly, but:

jsonlab seem okay, but it doesn't seem to be made to work well with other languages.   It seems to be made primarly for data storage, not data transmission.  Besides the fact that it's loaded with filesaving and compression (requiring external java library), it relies a lot on metadata.  For example, with 2D matrix, 'savejson.m' converts to a conventional JSON string, readily parsed to identical form in Javescript, python, etc., but not for ND arrays. For ND array, 'savejson' flattens the data into 1D array and includes metadata on array shape, variable name and class.  While still JSON compliant, it is not a conventional format for arrays, and require additional parsing on receiving end (i.e. non-trivial for Javascript, the JS in json).  

Also, setting aside the fact that I was unaware of the forthcoming 'jsonencode/decode', I also want to solve the problem of output data size and transmitting Inf and NaN, which I think neither 'jsonlab' nor 'jsonencode' really address properly.

First, 'toJSON' allows floating numbers to be truncated (precision), for more compact data transmission for large datasets.  For complex structured data, this is non-trivial either before or after encoding.

Second, I did not like their ('jsonlab', 'jsonencode', Firefox, Chrome, python, etc.) handling of Inf or NaN.   Either they turns both into 'null' (making them indistinguishable), or replace it with quoted-strings '"Infinity"' or '"NaN"'.  This require additional (computationally expensive for Javascript) parsing on receiving end.

The problem stems from the fact that JSON spec pretends to be IEEE754 complaint but doesn't actually make allowance for Infinity or NaN.  So, those numbers get trashed.  Instead 'toJSON' turns Inf into '1e999'.  While not a valid IEEE754 number, it automatically turns itself into Infinity when parsed by Javascript, Python, Ruby, Octave, Matlab, etc.  No fuss. No muss.  Nothing can be done about NaN, but with Infinity automatically parse, dealing with NaN is easier.

Likewise, 'fromJSON' cheats a little with regards to receiving Inf, it treats any floating number >= 1e308 as Inf.  Natively, Octave, JS, python, Ruby, etc. treat >= 2e308 as Inf, but by nudging this threshold down a tad, there's less work for transmitting end (JavaScript) to encode Inf, and it allows 'fromJSON' to very easily differentiate Inf from NaN.

Finally, 'toJSON' and 'fromJSON' handles complex number, which is slated to be unsupported in 'jsonencode' and 'jsondecode' (per MATLAB).  'jsonlab' uses more metadata.  Complex number are not native in Javascript nor supported by JSON, so structured object are the norm.  'jsonlab' does not follow this norm.  'toJSON', on the hand, handles complex number in a manner consistent with that norm. 'object2json' does similarly support complex numbers, however, where as it makes an array of structure object, 'toJSON' makes (for speed and output compactness) a structure object of arrays.  It's a design choice that super easy to change (add 2 lines, thanks to judicious use of recursion).  Additionally, 'object2json' is significantly (very!) slower, wrecks ND arrays, and there is no 'json2object'.  In contrast, 'fromJSON' reads in that structure object and converts it back into a Octave complex number. It works with either struct of array (small,fast) or array of struct (big,slow...for JSON).

When I make the requested change, do I upload it here again?  how do I name or version number the new files?

Anonymous
Fri 16 Oct 2020 07:28:26 PM UTC, comment #2: 

To reiterate what I wrote in the help-octave ML (typos fixed):

> In dev Octave we now have jsonencode and jsondecode functions which AFAICS are superior to object2json. I'll keep object2json in the io package for a while, to cater for users who have older Octave versions.
> In the mean time if you have JSON encode/decode functions that are better than what is now in io I'm interested.
> * Don't use the names jsonencode and jsondecode.
> * Be sure they're not depending on external libraries, just native Octave code.
> * At some time in the future the JSON functions in the io package will be deprecated. But that could take a few years ...
> * I'm not going to maintain them, that's up to you as author and maybe other volunteers. As package maintainer I'm just the first to make sure the package works, its functions have adequate documentation and no severe bugs.

So yes I'm not opposed to inclusion in the io package.
Other than Kai I'm not convinced that with Octave-7.0.0 next year your contributions would be obsolete; we regularly get bugs and question about ancient Octave versions that keep lingering on. Only when the io package no longer supports Octave < 7.x they'd be dropped.

Compared to JSONLAB, what do your functions offer? JSONLAB looks feature-complete, but I suppose there's a place for simple JSON I/O as well for upcoming Octave 6.1 and less.
What's the oldest Octave version your contributions will need? io currently supports Octave-4.2 so they should at least work there.

I can be a bit picky about contributions to packages I maintain (io and mapping), my quality demands are as follows, from more to less importance:

  1. Functions should have accurate output (and of course no obvious let alone severe bugs)
  2. Functions should be robust to wrong input (i.e., I like rigorous input validation)
  3. Functions should have adequate BIST tests if at all possible and practical, not only for their main function but also for input checks
  4. Octave coding style is strongly desired
  5. There should be a good Texinfo help text explaining all inputs and outputs of the functions.


Usually I'll polish the last two points if at least some attention was paid to them. Texinfo can be hard for novices so I don't mind helping out there.

Now, looking into toJSON.m and fromJSON.m:
1. (not checked yet, I don't use JSON myself). Can anyone help out here?
2. no input validation at all
3. excellent! yet input validation BIST tests are lacking ...
4. also excellent!
5. most of the required things are present, but needs more structure.

So if you add input validation and BIST tests, and someone verifies proper operation, I suppose they'll make it into the io package. Maybe not upcoming 2.6.3 but the next one after that should be possible.

Philip Nienhuis <philipnienhuis>
Group Member
Fri 16 Oct 2020 07:55:14 AM UTC, comment #1: 

Thanks for the contribution.  As described in the help mailing-list

https://lists.gnu.org/archive/html/help-octave/2020-10/msg00017.html

and the a must read (bug #53100) before putting too much effort in this project.

1. Octave 7 will have fast Matlab compatible json(en/de)code functions.
2. There is already a well-tested comprehensive JSON library as pure M-File implementation. https://github.com/fangq/jsonlab
3. Your function might fill the gap before the Octave 7 release next year by
3a. Inclusion in the io package
3b. Creation of an own lightweight package https://github.com/gnu-octave/pkg-example


Assigning this bug to Philip to decide about inclusion into the OF io package (3a.).  For core Octave your contribution will not be regarded (1.).  At your option (3b.) is always possible.

Kai Torben Ohlhus <siko1056>
Group Member
Thu 15 Oct 2020 06:04:42 PM UTC, original submission:  

Here (toJSON.m) is an alternative to object2json.m, along with a matching json parser (fromJSON.m).  It's faster, isn't constrained by numeric class, and conditions Inf, NaN, and NA for error-free parsing on receiving end.    It handles ND arrays better as well.

On documentation, I tried but am having trouble with formatting the layout.

Anonymous

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

Attached Files
file #52583:  fromJSON_v2_7_4.m added by None (35KiB - text/x-objcsrc - clean up comments)
file #52584:  toJSON_v_2_7_4.m added by None (18KiB - text/x-objcsrc - clean up comments)
file #52576:  fromJSON.m added by philipnienhuis (35KiB - text/x-matlab)
file #52577:  toJSON.m added by philipnienhuis (18KiB - text/x-matlab)
file #52578:  fromJSON.diff added by philipnienhuis (24KiB - text/x-patch)
file #52579:  toJSON.diff added by philipnienhuis (3KiB - text/x-patch)
file #52551:  fromJSON_v2_7_3.m added by None (36KiB - text/x-objcsrc - updated 'fromJSON',JSON with invalid UTF-8, random_json_test)
file #52552:  dhs_keywords.json added by None (8KiB - application/json - updated 'fromJSON',JSON with invalid UTF-8, random_json_test)
file #52553:  random_json_test.m added by None (1KiB - text/x-objcsrc - updated 'fromJSON',JSON with invalid UTF-8, random_json_test)
file #52545:  jsontrials.mat added by philipnienhuis (1KiB - application/octet-stream)
file #52546:  xulstore.json added by philipnienhuis (688B - application/json)
file #52547:  readjson.m added by philipnienhuis (229B - text/x-matlab)
file #52528:  fromJSON_v2_7_2.m added by None (30KiB - text/x-objcsrc)
file #52524:  fromJSON_v2_7_1.m added by None (31KiB - text/x-objcsrc - bug (deep scan of structure for complex number) and typo fixes)
file #52525:  toJSON_v2_7_1.m added by None (18KiB - text/x-objcsrc - bug (deep scan of structure for complex number) and typo fixes)
file #52518:  toJSON_v2_7.m added by None (18KiB - text/x-objcsrc - bug fix, feature change, refactor, improve doc)
file #52519:  fromJSON_v2_7.m added by None (30KiB - text/x-objcsrc - bug fix, feature change, refactor, improve doc)
file #52501:  blocklist-gfx.json added by philipnienhuis (27KiB - application/json)
file #50183:  fromJSON_v2_5.m added by None (21KiB - text/x-objcsrc - tweaked texinfo)
file #50184:  toJSON_v2_5.m added by None (18KiB - text/x-objcsrc - tweaked texinfo)
file #50178:  fromJSON.m added by philipnienhuis (21KiB - application/octet-stream)
file #50179:  toJSON.m added by philipnienhuis (18KiB - application/octet-stream)
file #50180:  fromJSON.diff added by philipnienhuis (14KiB - application/octet-stream)
file #50181:  toJSON.diff added by philipnienhuis (12KiB - application/octet-stream)
file #50177:  fromJSON_v2_4.m added by None (21KiB - text/x-objcsrc - fixed stupidity (fromJSON_v_2_3 is wrong file) and the UTF-8 error fix (in fromJSON_v_2_2.m))
file #50176:  fromJSON_v2_3.m added by None (10KiB - text/x-objcsrc - fixed the UTF-8 error fix)
file #50174:  toJSON_v2_2.m added by None (18KiB - text/x-objcsrc - fix UTF-8 encoding issue; added more BIST and help)
file #50175:  fromJSON_v2_2.m added by None (21KiB - text/x-objcsrc - fix UTF-8 encoding issue; added more BIST and help)
file #50144:  fromJSON_BIST.tst added by philipnienhuis (18KiB - application/octet-stream)
file #50145:  toJSON_BIST.tst added by philipnienhuis (23KiB - application/octet-stream)
file #50136:  fromJSON_v2_1.m added by None (17KiB - text/x-objcsrc - updated to comply with Octave 7)
file #50137:  toJSON_v2_1.m added by None (14KiB - text/x-objcsrc - updated to comply with Octave 7)
file #50087:  fromJSON_v2.m added by None (17KiB - text/x-objcsrc - update files)
file #50088:  toJSON_v2.m added by None (14KiB - text/x-objcsrc - update files)
file #49987:  toJSON.m added by None (9KiB - text/x-objcsrc)
file #49988:  fromJSON.m added by None (11KiB - text/x-objcsrc)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by mmuetzel (Posted a comment)
  • -email is unavailable- added by jwe (Posted a comment)
  • -email is unavailable- added by siko1056 (Posted a comment)
  • -email is unavailable- added by None (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 25 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2022-01-01 philipnienhuis Attached File- Added mxe_io_JSON_BIST_v2.patch, #52596
    2022-01-01 philipnienhuis Attached File- Added mxe_io_JSON_BIST.patch, #52594
    2021-12-29 philipnienhuis StatusIn Progress Done
        Open/ClosedOpen Closed
    2021-12-29 None Attached File- Added fromJSON_v2_7_4.m, #52583
        Attached File- Added toJSON_v_2_7_4.m, #52584
    2021-12-28 philipnienhuis Attached File- Added fromJSON.m, #52576
        Attached File- Added toJSON.m, #52577
        Attached File- Added fromJSON.diff, #52578
        Attached File- Added toJSON.diff, #52579
    2021-12-22 None Attached File- Added fromJSON_v2_7_3.m, #52551
        Attached File- Added dhs_keywords.json, #52552
        Attached File- Added random_json_test.m, #52553
    2021-12-21 philipnienhuis StatusNeed Info In Progress
    2021-12-21 philipnienhuis Attached File- Added jsontrials.mat, #52545
        Attached File- Added xulstore.json, #52546
        Attached File- Added readjson.m, #52547
    2021-12-18 None Attached File- Added fromJSON_v2_7_2.m, #52528
    2021-12-18 None Attached File- Added fromJSON_v2_7_1.m, #52524
        Attached File- Added toJSON_v2_7_1.m, #52525
    2021-12-16 None Attached File- Added toJSON_v2_7.m, #52518
        Attached File- Added fromJSON_v2_7.m, #52519
    2021-12-13 philipnienhuis Attached File- Added blocklist-gfx.json, #52501
    2021-11-26 philipnienhuis StatusDone Need Info
        Open/ClosedClosed Open

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code