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

Bug/issue 51 array with empty cluster #52

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

nate-moehring
Copy link
Contributor

@nate-moehring nate-moehring commented Sep 12, 2024

Issue #51

@nate-moehring
Copy link
Contributor Author

nate-moehring commented Sep 26, 2024

One thing led to another, led to another, led to another, in order to resolve all of the unit test errors. This implementation works but I'm not happy with my workaround in the begin-array case of Parse Value. I haven't had time to look further into better solutions yet. The problem is the To Variant actually seems to be necessary to satisfy the Deserialized Mixed Arrays unit tests. I don't understand how JSON can support arrays with elements of different datatypes, but if that is a requirement, an error is thrown by the Flattened String to Variant at the end of Array of VData to VArray__ogtk.vi with the To Variant wrapping the disparate array elements.

I'll put some more time into this tomorrow for a few hours, but after that, it's going to be difficult for me to get more time on this for a while. Do you have any directed pointers for me?

@nate-moehring
Copy link
Contributor Author

nate-moehring commented Sep 26, 2024

This is the list of errors that I get if I disable the current workaround of the To Variant loop-on-error after the Array of VData to VArray__ogtk.vi. With the workaround there are no unit test failures.
image

This list of failures was much longer before I implemented the integer array look-ahead inspection.

@nate-moehring nate-moehring force-pushed the bug/Issue-51-array-with-empty-cluster branch from 16c4cb6 to 5ac2a51 Compare September 29, 2024 23:45
@nate-moehring
Copy link
Contributor Author

@jimkring , I squashed the commits, hoping you have some ideas for better implementation.

@nate-moehring
Copy link
Contributor Author

@jimkring , any chance you've had a moment to pull the request?

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 this pull request may close these issues.

1 participant