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

refactor: replace 3 ak.from_json* functions with one #1617

Merged
merged 10 commits into from Aug 23, 2022

Conversation

jpivarski
Copy link
Member

@jpivarski jpivarski commented Aug 22, 2022

Unifying ak.from_json (in-memory), ak.from_json_file, and ak.from_json_schema.

  • verify that ak.to_json is up-to-date
  • verify that ak_from_json_new was actually finished way back in May
  • update tests
  • integrate the with-schema case into this function

All of the from_json cases go to RapidJSON for the parsing and are incremental in the JSON parsed (doesn't load a big string into memory, though it may result in a big ak.Array). For line-delimited JSON, a line_delimited=True argument is now necessary. Passing a file name now requires a pathlib.Path, rather than a bare str (which could be JSON text). All file/non-file cases are wrapped as a Python file-like object for reading in RapidJSON, which releases the GIL at all times except when reading from that file-like object.

Technically, the "refactor" commit type might not be correct because this PR is changing the API, but it's more than just adding a "feature" and it's not a "fix".

The new implementations were mostly copied from the following files, now deleted:

The last file, in turn, was developed before it as moved; the original location was:

@codecov
Copy link

codecov bot commented Aug 22, 2022

Codecov Report

Merging #1617 (bfbc295) into main (9e17f29) will increase coverage by 0.17%.
The diff coverage is 65.66%.

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%> (ø)
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_type.py 53.84% <0.00%> (ø)
src/awkward/_v2/_connect/cling.py 24.90% <2.73%> (-1.10%) ⬇️
src/awkward/_v2/operations/ak_transform.py 8.62% <8.62%> (ø)
... and 34 more

@jpivarski jpivarski linked an issue Aug 23, 2022 that may be closed by this pull request
@jpivarski
Copy link
Member Author

Finally closes #192!

@jpivarski
Copy link
Member Author

One thing I forgot: it's still missing documentation. (Not this PR.)

@jpivarski jpivarski enabled auto-merge (squash) August 23, 2022 03:30
@jpivarski jpivarski merged commit 9c33c2a into main Aug 23, 2022
@jpivarski jpivarski deleted the jpivarski/get-to_json-from_json-straightened-out branch August 23, 2022 03:41
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.

ak.to_json/ak.from_json should be revamped/organized for v2
1 participant