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

Error 1527 when parsing an array with cluster containing empty cluster and scalar #51

Open
jimkring opened this issue Sep 6, 2024 · 14 comments

Comments

@jimkring
Copy link
Contributor

jimkring commented Sep 6, 2024

Reported here.

Here is the reproducer:

Error 1527 JKI JSON.vi

image

{
    "array": [
        {
            "cluster": {
                "empty_cluster": {}
            },
            "scalar": 0
        }
    ]
}
@jimkring
Copy link
Contributor Author

jimkring commented Sep 6, 2024

@nate-moehring would you (or anyone else) be interested in looking into this issue?

  1. Create fork/branch for a Pull Request
  2. Add reproducer as a Caraya test with Issue 51 in VI name
  3. Fix the bug and keep all unit tests passing :)

@jimkring jimkring pinned this issue Sep 6, 2024
@nate-moehring
Copy link
Contributor

Sure, I can help with this, but first help me understand what the expected behavior would be. If you wired this to a Variant to Data node, the type cluster constant would need to have both 'empty_cluster' and 'cluster' elements omitted, otherwise the conversion would fail. Does SlippinJimmy know that these elements will always be empty? If not, then this error seems appropriate (though vague). If it's sometimes populated in jsonText, then the _type_cluster would need to be fully defined. Is there a way to specify optional (non-critical elements) of a json cluster such that isn't doesn't break the unflatten? If he does know that 'empty_cluster' is always empty, which therefore implies that 'cluster' will also be empty, then that seems like a pretty tough case to handle.

Workaround: Using the Path input on Native JSON Unflatten from String nodes to decode select items from the JSON is super easy and powerful.

@jimkring
Copy link
Contributor Author

jimkring commented Sep 7, 2024

@nate-moehring - that’s a great question.

I noticed that parsing this JSON (snippet below) seems to work OK (meaning: No Error and Variant data is returned successfully).

        {
            "cluster": {
                "empty_cluster": {}
            },
            "scalar": 0
        }

The error condition seems to occur in the specific case where the above snippet is an element of an array.

I’m not in front of my computer now or for most of today, but I’m thinking we should look at what the variant structure looks like for the similar, “working” cases.

A first/incremental step forward probably looks like “fixing” the inconsistency by getting the error to go away so that the test case outputs a variant with an array with one element that’s equivalent to the data produced from that second example JSON snippet.

Of course, the devil is in the details around how flattened data is handled. LabVIEW gets a bit finicky with zero element clusters since they aren’t valid edit-time/static data (and only make sense in dynamically-typed data (variants).

Where we may end up with this is: returning a null variant in liu of empty clusters ({}).

I’ll think of a bit more about this once I have a chance to get in front of the computer later this weekend.

@jimkring
Copy link
Contributor Author

jimkring commented Sep 8, 2024

@nate-moehring here's an example showing the desired output. The upper/working code shows how if we remove the outer array, we can successfully convert it to variant and then use the OpenG Variant library to make it an element of a 1D Array.

Example VI: Issue 51 - Array of Cluster with Element of Empty Cluster.vi

Example Screenshot:
image

@nate-moehring
Copy link
Contributor

I made some progress tonight. I used your repro to create a test VI (see my branch), and then found that I could fix this particular bug by removing a single node, but this then breaks other tests:
image

@nate-moehring
Copy link
Contributor

Without this change, this is the Variant data string that is breaking Array of VData to VArray, if you're fluent in such things:
'Value': Variant <Variant : {{{...}}, 0}>

2000 8000 0000 0004 0014 4050 0000 0D65 6D70 7479 5F63 6C75 7374 6572 0010 4050 0001 0000 0763 6C75 7374 6572 000D 4008 0006 7363 616C 6172 0000 0A00 5000 0200 0100 0200 0100 0300 0000 0000 0000 0000 0000 00

@nate-moehring
Copy link
Contributor

nate-moehring commented Sep 12, 2024

On the left side of the To Variant it has all the type-info, but on the right side it's like the type info gets lost, even though the data is preserved. (Yes, Show Type is enabled)
image

Perhaps this To Variant is essentially wrapping the variant in another variant and the Type Info display doesn't decode nested variants?

A: Yes, it does appear to be wrapping a fresh variant around it. When I use Variant to Data with a Type input of Variant, I get the original readable variant back. I always thought that wiring a variant into To Variant was a noop.

@nate-moehring
Copy link
Contributor

nate-moehring commented Sep 12, 2024

This repro shows the problem, any ideas? Report to NI?
Errored Variant.zip
image

@jimkring
Copy link
Contributor Author

@nate-moehring Yes, I would call this a LabVIEW bug -- nice work catching it!

Yes, let's report it to NI. Would you be willing to do that and report back here with an NI Bug ID #?

Regarding a short-term work-around, I'd be curious to see which cases now fail. Maybe the extra layer of variant wrapping in Parse Value could be removed and (whatever it was attempting to address could be) done in a different way.

Also, please create a pull request from your own fork/branch, since that'll make it easy for me to check it out and fiddle in there with you, if needed.

@jimkring
Copy link
Contributor Author

@nate-moehring actually, I'll ping LabVIEW R&D abound this, directly, and see if they can get it into the system.

@nate-moehring
Copy link
Contributor

@jimkring Pull request: #52

@francois-normandin
Copy link
Collaborator

francois-normandin commented Sep 15, 2024

Keeping an interested eye on this.
I used to rely on this library because jsonText does not accept deserialization without specifying type, but have since made a jsonText wrapper that supports it.

I'm attaching it here for anyone wanting to reverse engineer it and see how it could fix this Parser in the 0-element Cluster use case.

(VI depends on jsonText and LVOS' Data Manipulation)
JSON text to Variant (Void).zip

edit:
Just to be clear, OpenVariant.lvlib is 0-BSD licensed. If anything makes sense, just extract it and use. No dependency or attribution needed.

@nate-moehring
Copy link
Contributor

Thanks @francois-normandin . I've poked around with the libraries you provided, man that's a lot of code. In some ways it's more simple code, which is good, and it seems like it might handle more cases than the JKI library, but I'd have a hard time creating a side-by-side feature comparison list.

It's true that your library parses the problematic json just fine. It's also true that the array of variants that gets created just prior to being reshaped into a variant array works just fine. And an element of that array can be flattened and unflattened just fine, unless I force it through a To Variant like what is happening in Parse Value, then I see the same Error 1527.

My conclusion thus far is that I need to remove the To Variant from the JKI Parse Value begin-array case, and then look to the non-array cases of JSON text to Variant (Void) to help address the many resulting unit test errors.

@nate-moehring
Copy link
Contributor

My conclusion thus far is that I need to remove the To Variant from the JKI Parse Value begin-array case, and then ... address the many resulting unit test errors.

I should have some time to work on this next week.

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

No branches or pull requests

3 participants