sagemath / sage

Main repository of SageMath
https://www.sagemath.org
Other
1.47k stars 487 forks source link

Package libspatialindex, rtree #32000

Open DRKWang opened 3 years ago

DRKWang commented 3 years ago

Rtree is a ctypes Python wrapper of libspatialindex that provides a number of advanced spatial indexing features for the spatially curious Python user. These features include:

Nearest neighbor search

Intersection search

Multi-dimensional indexes

Clustered indexes (store Python pickles directly with index entries)

Bulk loading

Deletion

Disk serialization

Custom storage implementation (to implement spatial indexing in ZODB, for example).

Although it is named as Rtree, it also provides options for its variants in terms of splitting. There are 3 ways for it, that is, linear, quadratic, and R* tree splitting. The difference of them can be found here https://en.wikipedia.org/wiki/R-tree.

We create a normal package for Rtree.

In addition, CGAL also provides an implementation called "Intersecting Sequences of dD Iso-oriented Boxes" for the R tree, https://doc.cgal.org/latest/Box_intersection_d/index.html#Chapter_Intersecting_Sequences_of_dD_Iso-oriented_Boxes.

CC: @mkoeppe

Component: packages: optional

Keywords: Rtree, spatial index

Author: Binshuai Wang, Matthias Koeppe

Branch/Commit: public/32000 @ dfc5410

Issue created by migration from https://trac.sagemath.org/ticket/32000

DRKWang commented 3 years ago

Description changed:

--- 
+++ 
@@ -26,5 +26,6 @@
 - latest documentation: https://rtree.readthedocs.io/en/latest/
 - `libspatialindex` library: https://libspatialindex.org/en/latest/

+In addition, 
+CGAL also provides an implementation called "Intersecting Sequences of dD Iso-oriented Boxes" for the R tree, https://doc.cgal.org/latest/Box_intersection_d/index.html#Chapter_Intersecting_Sequences_of_dD_Iso-oriented_Boxes.

-
DRKWang commented 3 years ago

Branch: public/32000

mkoeppe commented 3 years ago
comment:4

The branch does not exist - did you push it?

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Commit: a60179a

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

mkoeppe commented 3 years ago
comment:6

This branch does not have your new files.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from a60179a to eb62e8a

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

c18ed02add new package rtree and libspatialindex
eb62e8aFor updating public/32000
mkoeppe commented 3 years ago
comment:8

the .txt files - the file names should all be lowercase. See the files for the existing packages. You can see the names that we use by typing grep SYSTEM= tox.ini We do not distinguish the various Debian variants (there should be no separate file for ubuntu, devuan, etc.)

First line of SPKG.rst should have a brief description, see format for other packages

mkoeppe commented 3 years ago
comment:9

checksums.ini - please add a line upstream_url as explained here https://doc.sagemath.org/html/en/developer/packaging.html#upstream-urls

mkoeppe commented 3 years ago
comment:10

build/pkgs/rtree/SPKG.rst is referring to "AABBTree", that's probably a copy-paste error

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from eb62e8a to c552a1e

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

c552a1ebuild/pkgs/libspatialindex: Make it a normal package
mkoeppe commented 3 years ago
comment:12

I've added full packaging as a normal package to libspatialindex because homebrew does not provide a package for it

mkoeppe commented 3 years ago

Author: Binshuai Wang, Matthias Koeppe

mkoeppe commented 3 years ago
comment:13

The file build/pkgs/rtree/requirements.txt needs to be removed because it makes the package a "pip package", so checksums.ini is being ignored

mkoeppe commented 3 years ago
comment:14

Does rtree use the executables mvrtree from libspatialindex, or does it link to library? In the latter case, the spkg-configure.m4 should be changed to use AC_LINK_IFELSE. You can see a usage example in build/pkgs/ntl/spkg-configure.m4

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2330ca9add new package rtree and libspatialindex
5902e3cbuild/pkgs/libspatialindex: Make it a normal package
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from c552a1e to 5902e3c

DRKWang commented 3 years ago
comment:16

Replying to @mkoeppe:

I've added full packaging as a normal package to libspatialindex because homebrew does not provide a package for it

I just found that homebrew provides a package for libspatialindex but it is in a different name: spatialindex, which is why I did not add it in .distros. You may check it from here https://repology.org/project/spatialindex/versions#homebrew

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

791aafdupdate SPKG.rst, correct distros, add url in checksums.ini, revise spkg-configure
7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 5902e3c to 791aafd

DRKWang commented 3 years ago
comment:18

Replying to @mkoeppe:

The file build/pkgs/rtree/requirements.txt needs to be removed because it makes the package a "pip package", so checksums.ini is being ignored

I have deleted it.

DRKWang commented 3 years ago
comment:19

Replying to @mkoeppe:

the .txt files - the file names should all be lowercase. See the files for the existing packages. You can see the names that we use by typing grep SYSTEM= tox.ini We do not distinguish the various Debian variants (there should be no separate file for ubuntu, devuan, etc.)

First line of SPKG.rst should have a brief description, see format for other packages

The names of those files in .distros have been changed following this format.

DRKWang commented 3 years ago
comment:20

Replying to @mkoeppe:

checksums.ini - please add a line upstream_url as explained here https://doc.sagemath.org/html/en/developer/packaging.html#upstream-urls

A downloading link from github release has been added into checksums.ini.

DRKWang commented 3 years ago
comment:21

Replying to @mkoeppe:

Does rtree use the executables mvrtree from libspatialindex, or does it link to library? In the latter case, the spkg-configure.m4 should be changed to use AC_LINK_IFELSE. You can see a usage example in build/pkgs/ntl/spkg-configure.m4

The spkg-configure.m4 has been revised following reference.

mkoeppe commented 3 years ago
comment:22

Replying to @DRKWang:

Replying to @mkoeppe:

the .txt files - the file names should all be lowercase. See the files for the existing packages. You can see the names that we use by typing grep SYSTEM= tox.ini We do not distinguish the various Debian variants (there should be no separate file for ubuntu, devuan, etc.)

First line of SPKG.rst should have a brief description, see format for other packages

The names of those files in .distros have been changed following this format.

lowercase file names please. It matters on case-sensitive file systems...

DRKWang commented 3 years ago
comment:23

Replying to @mkoeppe:

Replying to @DRKWang:

Replying to @mkoeppe:

the .txt files - the file names should all be lowercase. See the files for the existing packages. You can see the names that we use by typing grep SYSTEM= tox.ini We do not distinguish the various Debian variants (there should be no separate file for ubuntu, devuan, etc.)

First line of SPKG.rst should have a brief description, see format for other packages

The names of those files in .distros have been changed following this format.

lowercase file names please. It matters on case-sensitive file systems...

The names of files have also been modified for lowercase. But I forgot to mention it.

mkoeppe commented 3 years ago
comment:24

Replying to @DRKWang:

lowercase file names please. It matters on case-sensitive file systems...

The names of files have also been modified for lowercase. But I forgot to mention it.

This change is not on the branch - please push it

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 791aafd to 720ff79

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

68735cbtemporarily rename the distributions files under package rtree and libspatialindex with adding prefix_ for later renaming those files with all lowercase, this is caused by git did not make case-sensitive.
720ff79Rename the distribution files under package rtree and libspatialindex with lowercases.
DRKWang commented 3 years ago
comment:26

Replying to @mkoeppe:

Replying to @DRKWang:

lowercase file names please. It matters on case-sensitive file systems...

The names of files have also been modified for lowercase. But I forgot to mention it.

This change is not on the branch - please push it.

The lastest pushed branch has been changed for lowercase. Because the git is not case-sensitive, it still kept the same capital letters of those distribution files, even though I had changed them on my computer. After I realized the reason for that, I followed this solution https://stackoverflow.com/questions/17683458/how-do-i-commit-case-sensitive-only-filename-changes-in-git#1 and so that committed the code twice. Somehow, it is sort of not smart, but I did not figure out a better method for that.

mkoeppe commented 3 years ago
comment:27

Thanks for fixing this - that was a tricky one.

DRKWang commented 3 years ago
comment:28

Replying to @mkoeppe:

Thanks for fixing this - that was a tricky one.

No problem, thanks for your encouragement.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 720ff79 to 674d982

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. New commits:

594bb26make a whole change for RealSet class with using 'self.__class__' and '@classmethod', as well as adding a new subclass RealSet_rtree.
dfc5410Merge branch 'public/32000' of trac.sagemath.org:sage into realset_rtree
674d982"Fixed 2 bugs:
DRKWang commented 3 years ago
comment:31

I just made a wrong commit, which is 674d982, for this ticket. Is there any way to retract it?

mkoeppe commented 3 years ago
comment:32

git reset --hard HEAD~ takes away the top commit on your branch.

Using git push --force you can send the updated branch to the remote.

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

7ed8c4ca-6d56-4ae9-953a-41e42b4ed313 commented 3 years ago

Changed commit from 674d982 to dfc5410

DRKWang commented 3 years ago
comment:34

Replying to @mkoeppe:

git reset --hard HEAD~ takes away the top commit on your branch.

Using git push --force you can send the updated branch to the remote.

I see. Thanks!

mkoeppe commented 2 years ago
comment:37

Docbuild fails

OSError: /sage/local/var/lib/sage/venv-python3.10.2/lib/python3.10/site-packages/sage/geometry/cone.py:docstring of sage.geometry.cone.ConvexRationalPolyhedralCone.polar:6: WARNING: Bullet list ends without a blank line; unexpected unindent.
mkoeppe commented 2 years ago
comment:38

actually #32170

mkoeppe commented 1 year ago

Branch has merge conflicts