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

API: read-only access to GEOS pointer (__geom__, _geom, _ptr) #999

Closed
jorisvandenbossche opened this issue Oct 13, 2020 · 3 comments · Fixed by #1417
Closed

API: read-only access to GEOS pointer (__geom__, _geom, _ptr) #999

jorisvandenbossche opened this issue Oct 13, 2020 · 3 comments · Fixed by #1417

Comments

@jorisvandenbossche
Copy link
Member

Moving discussion from #983 (comment)

In current Shapely, there are two attributes that both return the GEOS pointer:

In [7]: from shapely.geometry import Point

In [8]: p = Point(1, 1)

In [9]: p
Out[9]: <shapely.geometry.Point POINT (1 1) >

In [10]: p.__geom__
Out[10]: 94617737200752

In [11]: p._geom
Out[11]: 94617737200752

In the branch subclassing from the C extension type (#983), there is now a third attribute inherited from PyGEOS that does the same:

In [12]: p._ptr
Out[12]: 94617737200752

I think it is fine to keep some way to get access to the GEOS pointer (eg if you want to use numba or cython yourself in some application where you know you are using the same GEOS version as Shapely, and thus passing along pointers can be safe). So we should maybe keep one of those attributes (although for cython, a function to get the GEOS pointer from a python object could also be used for this instead of an attribute).
But we don't necessarily need to keep all of them, and we can deprecate others?

As @mwtoews mentions at #983 (comment), __geom__ is not an official "dunder" method of python (and here also not used in a context for which typically dunder methods are defined), we could maybe deprecate __geom__ in Shapely 1.8, keeping _geom in Shapely 2.0 (since that is backwards compatible with older Shapely versions)?

cc @sgillies @caspervdw

@jorisvandenbossche
Copy link
Member Author

Note that in the shapely-2.0 branch, the attributes are actually read-only, unlike on current Shapely where you can set those _geom / __geom__ attributes yourself from python, and thus run into ugly errors.

@jorisvandenbossche
Copy link
Member Author

Concrete proposal for Shapely 1.8:

  • Deprecate accessing __geom__
  • Deprecate setting both __geom__ and _geom

Thoughts?

@mwtoews
Copy link
Member

mwtoews commented Nov 4, 2020

The proposed plan for __geom__ and _geom are reasonable.

Access to both attributes _geom and _ptr (as the same object) should be fine for Shapely 2+.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants