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

Create Python bindings? #29

Closed
arenas-guerrero-julian opened this issue Feb 13, 2023 · 11 comments
Closed

Create Python bindings? #29

arenas-guerrero-julian opened this issue Feb 13, 2023 · 11 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@arenas-guerrero-julian
Copy link

Hi,

Thanks for your work :)

To the best of my knowledge, python does not have a well maintained JSONPath library. Maybe it could be a good idea to create python bindings so that this library could be used from python.

@besok
Copy link
Owner

besok commented Feb 13, 2023

Hi.
Thank you!

That is a very interesting idea.
But it needs to take a close look at how to do that.
Maybe you have something in mind already?

@arenas-guerrero-julian
Copy link
Author

Hi,

Unfortunately I am not of much help here :( I just know it should be possible with pyo3.

@besok
Copy link
Owner

besok commented Feb 13, 2023

No worries. I will give it a look

@besok besok added enhancement New feature or request help wanted Extra attention is needed labels Mar 15, 2023
@night-crawler
Copy link
Contributor

Hey folks, I started some work on creating python bindings:
https://github.com/night-crawler/jsonpath-rust-bindings

maturin develop

python
>>> from jsonpath_rust_bindings import jsonpath
>>> from jsonpath_rust_bindings import Finder
>>> f = Finder({'a': {'b': 42}})
>>> f
<builtins.Finder object at 0x7f342d92cc90>
>>> f.find('$.a')
[<builtins.JsonPathResult object at 0x7f342d4fe2f0>]
>>> f.find('$.a')[0]
<builtins.JsonPathResult object at 0x7f342d4fe830>
>>> f.find('$.a')[0].data
{'b': 42}

There's a standing question in regards Rust -> Python && Python -> Rust PyObject conversion and cloning: https://github.com/night-crawler/jsonpath-rust-bindings/blob/main/src/lib.rs#L48

Is it possible to have a JsonPathFinder not owning a Value?

@besok
Copy link
Owner

besok commented Nov 5, 2023

Hey! Great to hear that! Thank you!
Is it possible to have a JsonPathFinder not owning a Value?

yeah, as far as I understand your question it is not possible since the given structure literally takes ownership of it.

pub struct JsonPathFinder {
    json: Box<Value>,
    path: Box<JsonPathInst>,
}

But this thing is just a helper structure to handle the given JSON (usually) from string and the given query(also usually from string).

You can try to avoid possession of the JSON value using a query directly.

  #[test]
    fn no_clone_api_test() {
        fn test_coercion(value: &Value) -> Value {
            value.clone()
        }

        let json: Value = serde_json::from_str(template_json()).expect("to get json");
        let query = JsonPathInst::from_str("$..book[?(@.author size 10)].title")
            .expect("the path is correct");

        let results = query.find_slice(&json);
        let v = results.get(0).expect("to get value");

        // V can be implicitly converted to &Value
        test_coercion(v);

        // To explicitly convert to &Value, use deref()
        assert_eq!(v.deref(), &json!("Sayings of the Century"));
    }

is it what you need?

@night-crawler
Copy link
Contributor

Great to hear that! Thank you!

Thank you too for your jsonpath efforts!

You can try to avoid possession of the JSON value using a query directly.

Right. In fact, I just wanted to have paths to the original extracted values returned always and got lost in the API a bit. I could use json_path_instance() function directly: https://github.com/night-crawler/jsonpath-rust-bindings/blob/main/src/lib.rs#L39

A side note and probably an off-topic. Currently, I'm converting PyObjects to Values with pythonize crate, and it seems that it clones the original values, which is not very efficient and has a side effect better demonstrated below:

Python 3.11.5 (main, Aug 30 2023, 19:09:52) [GCC 13.2.1 20230730] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> original_object_i_want_to_mutate = {'a': {'b': 'sample b'}}
>>> from jsonpath_rust_bindings import Finder
>>> f = Finder(original_object_i_want_to_mutate)
>>> b_dict = f.find('$.a')[0].data
>>> b_dict
{'b': 'sample b'}
>>> b_dict['new'] = 42
>>> original_object_i_want_to_mutate
{'a': {'b': 'sample b'}}

I wonder if it's wise to pursue it now, or save it for later. Any thoughts?

@besok
Copy link
Owner

besok commented Nov 5, 2023

Right. In fact, I just wanted to have paths to the original extracted values returned always and got lost in the API a bit. I could use json_path_instance() function directly: https://github.com/night-crawler/jsonpath-rust-bindings/blob/main/src/lib.rs#L39

yeah, that works as well :). Great.

A side note and probably an off-topic. Currently, I'm converting PyObjects to Values with pythonize crate, and it seems that it clones the original values, which is not very efficient and has a side effect better demonstrated below:

Yeah, I will need to take a look at how the crate serializes the objects, but at first glance, looking into this test or that one it seems possible that the data gets cloned.

But, I wonder how to avoid cloning between Rust and Python as we don't share one object but transfer the data (maybe, I don't grasp it clearly though).

Anyway, regarding data mutations for now, even if you had gotten the pointer to the slice of original data, you would not be able to modify it, at least, from rust. There are a couple of open issues: #48 and #12

I would proceed with cloning now and then think about how to avoid it but later on.

@night-crawler
Copy link
Contributor

So, here's the result: https://pypi.org/project/jsonpath-rust-bindings/

Regarding the conversion problem, there's already an issue but solutions mentioned in comments don't look like zero-copy to me. I'll allocate some time this week to see what else I can do with it.

And yet again, thank you!

@besok
Copy link
Owner

besok commented Nov 6, 2023

Very well! Thank you very much!
Yeah, the problem with cloning needs a warning but basically, it goes into serde problem.

Anyway, if you can spare some time lately, that would be nice if you add a couple of lines into the readme, mentioning binding.

@night-crawler
Copy link
Contributor

that would be nice if you add a couple of lines into the readme, mentioning binding.

I apologize for the confusion, probably pypi returned you an old cached version with an empty readme (v0.1.0). The current one 0.1.1 explicitly specifies the link to the jsonpath-rust project. Also, I updated the readme with your name just in case (not yet published to the pypi).

If I didn't get the meaning of mentioning binding, please follow up on this or DM me. Thank you.

@besok
Copy link
Owner

besok commented Nov 6, 2023

Ah sorry, I did mention it vaguely and that was the cause of the confusion. I meant the readme of this project.
Just a couple of lines that the current crate has a py binding with a link to it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants