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

Fix iter issues #7

Merged
merged 5 commits into from
Mar 5, 2025
Merged

Fix iter issues #7

merged 5 commits into from
Mar 5, 2025

Conversation

justmobilize
Copy link
Contributor

@justmobilize justmobilize commented Feb 26, 2025

This fixes some issues when grabbing the last key, or lists of single items when the , doesn't exist.

  1. added _IterToStream.last_char to allow Transient to see what it really ended on
  2. moved duplicated code from TransientList and TransientObject into the base class
  3. added Transient.as_object so you can get the entire object
  4. added tests (currently at 95% coverage)

@justmobilize justmobilize marked this pull request as ready for review February 26, 2025 23:43
@justmobilize
Copy link
Contributor Author

@tyeth and @Neradoc would love your thoughts.



def test_as_object_grabbing_multiple_subscriptable_levels_twice_raises(complex_dict):
"""Test loading a complex dict and grabbing multiple subscriptable levels twice raises."""
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this work not fail, assuming the keys are ordered and the user hasn't left [asked for later data] the current parent object?
If not then what is the correct usage, to request the parent object separately first and then subscript that?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tyeth challenge accepted and fixed: 1b47d3e

@justmobilize
Copy link
Contributor Author

If there are any other tests we can think to add, I'll put them in. Then use those tests to update docs as to what can and can't be done

@Neradoc
Copy link

Neradoc commented Feb 27, 2025

Thanks for working on that. Seems good to me.
I just have a small question: why check self.data.last_char for both } and ] in TransientObject and not just } ? TransientList only tests for ]. That wouldn't affect a well-formed json file anyway I believe, so it's not an issue.

@justmobilize
Copy link
Contributor Author

Good catch. I think I added it while working through things and it was before I had TransientList working. Changed it and all tests still pass.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for improving this! I really appreciate it. Just a couple small things.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One documentation request. Looks good otherwise. Thanks!

@justmobilize justmobilize requested a review from tannewt March 4, 2025 17:53
Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@tannewt tannewt merged commit b71c722 into adafruit:main Mar 5, 2025
1 check passed
@justmobilize justmobilize deleted the fix-iter-issues branch March 5, 2025 19:09
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.

4 participants