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

[python] Allow use of other JSON encoder/decoders #1654

Open
bollwyvl opened this issue Dec 13, 2022 · 4 comments
Open

[python] Allow use of other JSON encoder/decoders #1654

bollwyvl opened this issue Dec 13, 2022 · 4 comments

Comments

@bollwyvl
Copy link

Thanks for oso!

It would be lovely if there was a simple way for polar to make use of other, more performant JSON encoder/decoder libraries.

For example, by monkeypatching the rust-based orjson into polar.(cffi|query|errors), I've observed calls to json.loads pretty much disappearing into noise when profiled with pyinstrument, whereas previously it was rather pronounced.

@samscott89
Copy link
Member

Hey @bollwyvl!

Thanks for the suggestion. That sounds like a huge win. If it can be accomplished by monkeypatching, is it just a drop in replacement we could use? Do you know if it supports the same platforms/versions that we support?

Either way, sounds like having a feature like oso[orjson] as an opt-in feature could be helpful. We definitely rely pretty heavily right now on JSON encoding/decoding.

@bollwyvl
Copy link
Author

Welp, as oso is already rust-dependent, so I don't see how it could be much of a limitation. orjson use is already fairly widespread, with some plugin support in e.g. FastAPI, django. As to what specific wheels are available? No idea, as I primarily work with (and re-package oso for) conda-forge, where we have aarch64 and ppcle as the most "exotic" things. Could probably support Apple Silicon, but nobody has asked (and still building under rosetta).

But again, "picking a horse" might not be as effective as "leaving an open stable," and letting downstreams do their own shim around it, with proper docs and types of what it might ask for in e.g. polar.set_json_encoder or something.

This would leave the way for exploring options, dependent on the workload, e.g. pysimdjson, or whatever comes out next month. To my knowledge, there isn't a performant, async-compatible JSON encoder/decoder... but then oso doesn't do that much either.

@kkirsche
Copy link
Contributor

kkirsche commented Feb 2, 2023

orjson supports CPython 3.7, 3.8, 3.9, 3.10, and 3.11. It distributes x86_64/amd64, aarch64/armv8, and arm7 wheels for Linux, amd64 and aarch64 wheels for macOS, and amd64 wheels for Windows. orjson does not support PyPy. Releases follow semantic versioning and serializing a new object type without an opt-in flag is considered a breaking change.

Per https://pypi.org/project/orjson/

I personally agree that a more generic setter pattern (though I'd probably use a @property rather than a set_.... method which is more common in languages like Java) would probably be better. If this is of interest, I may be able to look at adding this if we can agree to functionally how this should be added.

Thanks 🙂

@bollwyvl
Copy link
Author

bollwyvl commented Feb 2, 2023

Well, if there's always "The One True" oso at any given time, it could just be in the constructor:

oso = Oso(
    json_loads: Callable[[Any], str] = json.loads, 
    json_dumps: Callable[[str], Any] = json.dumps
)

And it's up to the implementer to overload the default by subclass or instantiation

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

No branches or pull requests

3 participants