[2.0] Disallow setting custom attributes on geometry objects#1181
Merged
jorisvandenbossche merged 1 commit intoshapely:shapely-2.0from Sep 30, 2021
Merged
Conversation
Member
Author
|
Additional benefit of defining (from a quick test with |
Member
Author
|
I also opened a PR for the shapely 1.8 version with a deprecation: #1183 |
3498c0c to
bc2e464
Compare
7c3252b to
611452f
Compare
| self._coords = CoordinateSequence(coords_array) | ||
| return self._coords | ||
| coords_array = pygeos.get_coordinates(self, include_z=self.has_z) | ||
| return CoordinateSequence(coords_array) |
Member
Author
There was a problem hiding this comment.
We can see later if we want to "cache" this again (there might be other attributes you could cache as well)
611452f to
a393e99
Compare
a393e99 to
5ee42ec
Compare
jorisvandenbossche
added a commit
that referenced
this pull request
Oct 27, 2021
jorisvandenbossche
added a commit
that referenced
this pull request
Oct 28, 2021
jorisvandenbossche
added a commit
that referenced
this pull request
Nov 8, 2021
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
xref https://github.com/Toblerity/Shapely/pull/1180/files#r693378882
PyGEOS geometries are "immutable", not only regarding the coordinates of the geometry, but also the python object: you can't assign random attributes as you can do with a normal class defined in python (this is because they are C extension types):
However, because in Shapely we still create a Python-defined subclass for each of the geometry types (and register this, so the C code will actually instantiate those subclasses), we get the typical behaviour of standard python classes still being "mutable" objects:
It's however relatively easy to keep the "can't set custom attribute" behaviour of the base C class (if we want this), by adding an empty
__slots__to each of the python subclasses. (which is what this PR is doing)Personally, I liked the fact that pygeos Geometries were completely immutable, so I would be in favor of preserving this characteristic in Shapely as well.
(for example, we will generally re-use objects (eg when broadcasting) because they are immutable, and for such cases, setting attributes might give surprising behaviour)
If we would want to disallow setting attributes in Shapely 2.0, following PyGEOS behaviour, that's of course a breaking change compared to Shapely 1.x, but I think it should be relatively easy to add a deprecation warning for this change as well (adding a warning in
__setattr__).cc @sgillies @caspervdw