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: Ensure that ak._v2.to_json raises errors when appropriate. #1649

Merged

Conversation

jpivarski
Copy link
Member

While looking at to_list, I had some questions for myself about the to_json code right beside it, which seemed to not raise errors on bad JSON. The mistake was in my test: v1 has been silently generating bad JSON, not v2! But anyway, here are explicit tests for to_json error-checking so that this doesn't come up again (for v2 users).

@jpivarski jpivarski enabled auto-merge (squash) August 30, 2022 17:38
@codecov
Copy link

codecov bot commented Aug 30, 2022

Codecov Report

Merging #1649 (29679bc) into main (9e17f29) will increase coverage by 0.52%.
The diff coverage is 67.67%.

Additional details and impacted files
Impacted Files Coverage Δ
src/awkward/_v2/_connect/cuda/__init__.py 0.00% <0.00%> (ø)
src/awkward/_v2/_connect/numexpr.py 88.40% <0.00%> (ø)
src/awkward/_v2/_connect/pyarrow.py 88.46% <0.00%> (ø)
...awkward/_v2/_connect/rdataframe/from_rdataframe.py 0.00% <0.00%> (ø)
src/awkward/_v2/_lookup.py 98.68% <ø> (+1.17%) ⬆️
src/awkward/_v2/numba.py 93.47% <0.00%> (ø)
src/awkward/_v2/operations/ak_from_avro_file.py 66.66% <0.00%> (ø)
src/awkward/_v2/operations/ak_local_index.py 100.00% <ø> (ø)
src/awkward/_v2/operations/ak_max.py 87.09% <ø> (ø)
src/awkward/_v2/operations/ak_min.py 87.09% <ø> (ø)
... and 59 more

@jpivarski jpivarski merged commit 8da1671 into main Aug 30, 2022
@jpivarski jpivarski deleted the jpivarski/ensure-that-to_json-raises-errors-when-appropriate branch August 30, 2022 18:31
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