geocaml / ocaml-rtree

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

Add `Cube` bounding box like `Rectangle` #17

Closed patricoferris closed 11 months ago

patricoferris commented 1 year ago

The user is free to provide their own implementation for a bounding box. Currently the library provides a Rectangle as that would be quite common. We could also provide an implementation for cubes (3D rectangles).

Mankavelda commented 1 year ago

@patricoferris , I am new to this project. I just forked and clone the repo and I'm on the next steps to setting it up

patricoferris commented 1 year ago

Hi @Mankavelda once you have the project building I can assign you this issue, let me know when things are up and running. And do reach out if you encounter any problems.

Mankavelda commented 1 year ago

Hello @patricoferris thanks for reaching out. Actually I have ran the commands dune build and runtest.. But then I don't know how to run the code (like see the output)

khushishikhu commented 1 year ago

Hi @patricoferris I am outreachy applicant and if you don't mind can I take interest in working with this issue.

patricoferris commented 1 year ago

Hi @khushishikhu, @Mankavelda seems to be set up and ready to go, how about #25 ?

@Mankavelda great job getting setup, there shouldn't really be too much output when you run dune build as you are only building the library. After dune runtest you should see something like:

# Some JSON
...
Ran: 6 tests in: 0.01 seconds.
OK
khushishikhu commented 1 year ago

Hi @khushishikhu, @Mankavelda seems to be set up and ready to go, how about #25 ?

@Mankavelda great job getting setup, there shouldn't really be too much output when you run dune build as you are only building the library. After dune runtest you should see something like:

# Some JSON
...
Ran: 6 tests in: 0.01 seconds.
OK

Sure. No problem. All the best @Mankavelda

kushalpokharel commented 12 months ago

Hi @patricoferris I would like to work on this project and saw that there are no good-first-issue that are not assigned to anyone. Could you assign me any? Or do I go for the nearest neighbor issue?

AryanGodara commented 12 months ago

Hi @patricoferris I would like to work on this project and saw that there are no good-first-issue that are not assigned to anyone. Could you assign me any? Or do I go for the nearest neighbor issue?

@kushalpokharel The nearest neighbour issue has also been assigned to someone else :)) We'll try to find something for you as well

kushalpokharel commented 12 months ago

Thank you @AryanGodara, I am waiting.

patricoferris commented 12 months ago

@Mankavelda, how's everything going ? Anything you are stuck on ?

kushalpokharel commented 12 months ago

Hi @patricoferris, I was going through the codebase and found that the envelope has to have a function area which can be quite confusing for higher dimensions. Should we rename it or that is okay for now?

patricoferris commented 12 months ago

Good question, hmm for now let's go with Area and then we can decide in the PR once all the pieces are in place ?

kushalpokharel commented 11 months ago

@patricoferris , also to get hands dirty I was working on this issue. I have written code for Cube along with its tests. If @Mankavelda hasn't started the task, maybe I can push the code?

AryanGodara commented 11 months ago

@kushalpokharel unfortunately, the PR is still assigned to @Mankavelda and @harshey1103 already opened a PR for it, and we had to close it as well. :) Maybe you can share a link to the cube branch on your fork, I'd love to take a look at the implmentation

kushalpokharel commented 11 months ago

Sure @AryanGodara, this is my branch where I have implemented the cube bounding box

patricoferris commented 11 months ago

Nice @kushalpokharel ! Do open that as a PR and we can get cracking on the review and merge :))

Edit: I totally misread that, still great work @kushalpokharel, but this was already assigned to @Mankavelda. @Mankavelda how's this going, would you like to collaborate with @kushalpokharel on this, perhaps you could review the PR with me if we open one based on the branch above?

harshey1103 commented 11 months ago

Hi @patricoferris. I had previously initiated a pull request for the same issue, which was subsequently closed since the issue had already been assigned to another contributor. Would it be possible for me to also contribute to the resolution of this issue?

kushalpokharel commented 11 months ago

I have added a PR for this. We can all review and contribute to it. Is that okay? @harshey1103 @Mankavelda @patricoferris

AryanGodara commented 11 months ago

Hi @kushalpokharel, it's good that you have an implementation. But you should've waited before submitting a PR on this, as the issue is already assigned to someone else :) Let's focus on your other open PR first, then move on to this one

patricoferris commented 11 months ago

Hmm this got a little messy, I agree with @AryanGodara about PRs for issues that have not been assigned. However, we are where we are which is my fault. I suggest that @harshey1103 and @Mankavelda review the open PR and can then use that PR as part of their application. Apologies again for not keeping on top of this.

Mankavelda commented 11 months ago

Hmm this got a little messy, I agree with @AryanGodara about PRs for issues that have not been assigned. However, we are where we are which is my fault. I suggest that @harshey1103 and @Mankavelda review the open PR and can then use that PR as part of their application. Apologies again for not keeping on top of this.

Hello @patricoferris , thank you. I will review the PR

Mankavelda commented 11 months ago

@Mankavelda, how's everything going ? Anything you are stuck on ?

Hello @patricoferris No issue for now

kushalpokharel commented 11 months ago

Hi, sorry guys. I picked this up when I had no ticket and since I had solved this already I just wanted to show my efforts. Apologies

Mankavelda commented 11 months ago

Sure @AryanGodara, this is my branch where I have implemented the cube bounding box

Hello @kushalpokharel please how do I run this to see the output?

Mankavelda commented 11 months ago

Please can someone help me out? I am facing this error on trying to compile Screenshot from 2023-10-17 08-51-14

Mankavelda commented 11 months ago

Hi @khushishikhu, @Mankavelda seems to be set up and ready to go, how about #25 ?

@Mankavelda great job getting setup, there shouldn't really be too much output when you run dune build as you are only building the library. After dune runtest you should see something like:

# Some JSON
...
Ran: 6 tests in: 0.01 seconds.
OK

Thanks @patricoferris , I have done that. But then how do I go on to run and see the output of the code changes I made?