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.

Sign up for GitHub

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

Jump to bottom

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 •
edited
Loading

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.

tyeth
tyeth reviewed Feb 26, 2025
View reviewed changes
tests/test_json_stream.py Outdated


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
Contributor

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.

tannewt
tannewt requested changes Feb 27, 2025
View reviewed changes
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.

tannewt
tannewt requested changes Mar 4, 2025
View reviewed changes
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
tannewt
tannewt approved these changes Mar 5, 2025
View reviewed changes
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
@justmobilize justmobilize mentioned this pull request Mar 6, 2025
Open
adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request Mar 13, 2025
@adafruit-adabot
Automated update by Adabot (adafruit/adabot@761055f)
8272863 
Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SH1107 to 2.0.1 from 2.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_SH1107#27 from FoamyGuy/use_ruff_remove_8x_compatibility

Updating https://github.com/adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306 to 3.0.1 from 3.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_DisplayIO_SSD1306#51 from FoamyGuy/use_ruff_and_remove_8x_compat

Updating https://github.com/adafruit/Adafruit_CircuitPython_SSD1680 to 2.0.1 from 2.0.0:
  > Merge pull request adafruit/Adafruit_CircuitPython_SSD1680#31 from makermelissa/main

Updating https://github.com/adafruit/Adafruit_CircuitPython_JSON_Stream to 0.8.6 from 0.8.5:
  > Merge pull request adafruit/Adafruit_CircuitPython_JSON_Stream#7 from justmobilize/fix-iter-issues

Updating https://github.com/adafruit/Adafruit_CircuitPython_MiniMQTT to 8.0.0 from 7.11.6:
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#247 from adafruit/ruff-format-reconnect-tests
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#244 from vladak/reconnect_restoration
  > Merge pull request adafruit/Adafruit_CircuitPython_MiniMQTT#240 from vladak/exceptions_tidy_up
@Neradoc Neradoc mentioned this pull request Mar 19, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tannewt tannewt

@tyeth tyeth

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@justmobilize @Neradoc @tannewt @tyeth