geocaml / ocaml-rtree

A pure OCaml implementation of R-Trees
BSD 3-Clause "New" or "Revised" License
26 stars 7 forks source link

Provide `iter` function #19

Closed patricoferris closed 8 months ago

patricoferris commented 10 months ago

This function should look something like:

val iter : Rtree.t -> (Rtree.tree -> unit) -> unit

The order doesn't matter and we can add that there is no guarantees on this.

Lukong123 commented 9 months ago

Hello, @patricoferris I would love to work on this issue.

patricoferris commented 9 months ago

Hey @Lukong123, nice to have you here! Have you got OCaml setup and the project building?

Lukong123 commented 9 months ago

Hey @Lukong123, nice to have you here! Have you got the OCaml setup and the project building?

Thanks for having me here too. I succeeded in building and testing the project I also have OCaml running on my machine. Even though I am new to Ocaml I'm willing to learn in order to make meaningful contributions.

AryanGodara commented 9 months ago

@Lukong123 are you working on this issue? You can always ask for help if you're having any issues :)

Bash-dot commented 9 months ago

@Lukong123 Hello, I noticed you mentioned you have Ocaml running on your system and you succeeded in building and testing the project, id appreciate if you could guide me through the process, is there anyway we can communicate

Lukong123 commented 9 months ago

@Lukong123 Hello, I noticed you mentioned you have Ocaml running on your system and you succeeded in building and testing the project, id appreciate if you could guide me through the process, is there anyway we can communicate

Hello @Bash-dot , sure I will be glad to help and yes we can communicate on Discord.
Once you have Ocaml installed you can quickly build and test this project by running

Lukong123 commented 9 months ago

@Lukong123 are you working on this issue? You can always ask for help if you're having any issues :)

Yes, I am working on this issue. It's taking me a while because I am still new to the language and I was trying to understand some things on it.

Lukong123 commented 9 months ago

Hello @patricoferris @AryanGodara I am having some errors when running dune build I want to ask having this code

let rec iter (rtree : Rtree.t) (f : Rtree.tree -> unit) : unit = match rtree with | Rtree.Leaf -> ()
| Rtree.Node (value, children) -> f value;
List.iter (fun child -> iter child f) children

inside of rtree.ml file I am having error unbound module Rtree I don't know what might be causing this.

patricoferris commented 9 months ago

Good question @Lukong123. The module prefix (Rtree) is often needed in code that uses Rtree as a library to tell where the constructors (Leaf, Node) are coming from. Because we are inside the module itself then Rtree doesn't refer to anything. You should be able to remove the module prefix provided the constructor are in scope. Does that make sense? I left some review comments on the PR :))

Lukong123 commented 9 months ago

Good question @Lukong123. The module prefix (Rtree) is often needed in code that uses Rtree as a library to tell where the constructors (Leaf, Node) are coming from. Because we are inside the module itself then Rtree doesn't refer to anything. You should be able to remove the module prefix provided the constructor are in scope. Does that make sense? I left some review comments on the PR :))

Thanks I saw the review on the PR. It makes a lot of sense now thank you very much.

Lukong123 commented 9 months ago

Hello @AryanGodara please I need help I currently have the iter function as such

let rec iter rtree f = match rtree with | Leaf value -> f value | Node children -> f Node ; List.iter (fun (_, child) -> iter child f) children | Empty -> f

my current aim is to have call f on the Node itself and then recurse to the children.

Node children -> f Node ; List.iter (fun (_, child) -> iter child f) children

calling f Node I expect it to call f on the Node before recursively iterating over its children. But I have an error on that line which says

f Node ; ^^^^

Error: This variant expression is expected to have type (Value.envelope * Value.t) list The constructor Node does not belong to type list

Lukong123 commented 9 months ago

Hello @AryanGodara please I need help I currently have the iter function as such

let rec iter rtree f = match rtree with | Leaf value -> f value | Node children -> f Node ; List.iter (fun (_, child) -> iter child f) children | Empty -> f

my current aim is to have call f on the Node itself and then recurse to the children.

Node children -> f Node ; List.iter (fun (_, child) -> iter child f) children

calling f Node I expect it to call f on the Node before recursively iterating over its children. But I have an error on that line which says

f Node ; ^^^^ Error: This variant expression is expected to have type (Value.envelope * Value.t) list The constructor Node does not belong to type list

I am confused how to properly loop through the node including the current node

patricoferris commented 9 months ago

Did the comment I left on the PR help with this @Lukong123 ?

Lukong123 commented 9 months ago

Hello sir @patricoferris @AryanGodara , Thanks @patricoferris I have looked into the comments and right now I am working on the test for the iter function. Though I am still facing some difficulties , I have my test as

let test_iter () = let module R = Rtree.Make (Rtree.Rectangle) (struct type t = int

        let t = Repr.float

        let envelope { Rtree.Rectangle.x_min = x1; y_min = y1; x_max = x2; y_max = y2 } =
          let x0 = min x1 x2 in
          let x1 = max x1 x2 in
          let y0 = min y1 y2 in
          let y1 = max y1 y2 in
          Rtree.Rectangle.v ~x_min:x0 ~y_min:y0 ~x_max:x1 ~y_max:y1
      end)
  in

  let myTree =
    R.Node [
      ((1, 2), R.Leaf [((1, 2)); (3, 4)]);
      ((2, 3), R.Leaf [((5, 6)); (7, 8)]);
    ]
  in
  let square_point (x, y) =
    let squared_x = x * x in
    let squared_y = y * y in
    let _squared_point = (squared_x, squared_y) in
    Printf.printf "Squared Point: (%d, %d)\n" squared_x squared_y;
    let expected_squared_x = x * x in
    let expected_squared_y = y * y in
    assert (squared_x = expected_squared_x && squared_y = expected_squared_y);
    squared_x, squared_y
  in
  R.iter myTree square_point

but I have an error

let envelope { Rtree.Rectangle.x_min = x1; y_min = y1; x_max = x2; y_max = y2 } = ^^^^^^^^^^^^^^^^^^^^^ Error: Unbound record field Rtree.Rectangle.x_min

I am working such that it should be similar to the other test in the test file but I have this error. Since I have been trying to sort it out, I came across a suggestion suggesting that there is a mismatch between the envelope type used in the Rtree module creation and the envelope type used in the iter function. I have been on it for some hours and am not yet getting it. Please can you provide me with some clarifications?

patricoferris commented 8 months ago

Fixed in #24