[2.0] Remove mutability of geometries #960
Conversation
|
@jorisvandenbossche done. You still need to rebase your own branch, right? |
|
Thnks! Switching branches back and again also seemed to trigger GitHub |
…remove-mutability
|
|
||
| def _set_coords(self, ob): | ||
| raise NotImplementedError( | ||
| "set_coords must be provided by derived classes") |
There was a problem hiding this comment.
Bye bye fragile mutable geometries 🔪
sgillies
left a comment
There was a problem hiding this comment.
I love the +/- ratio on this PR: 1/132 🥇
There was a problem hiding this comment.
Theses changes are welcome! I have suggestions to keep (and modify) two tests to check that point and linestring geometries now have immutable .coords properties. It might be helpful to the reader to see a comment to say that these were mutable in previous versions. All other tests should be dropped, however.
Good point, will update for that. Thanks for the review! |
|
Added back tests for point/linestring/linearring to ensure immutability. We now have those errors: In [1]: from shapely.geometry import LineString
In [3]: l = LineString([(0,0), (1,1)])
In [6]: l.coords[0] = (5, 5)
---------------------------------------------------------------------------
TypeError Traceback (most recent call last)
<ipython-input-6-41687dadde5d> in <module>
----> 1 l.coords[0] = (5, 5)
TypeError: 'CoordinateSequence' object does not support item assignment
In [7]: l.coords = [(0,0), (1,1)]
---------------------------------------------------------------------------
AttributeError Traceback (most recent call last)
<ipython-input-7-dc6ae96a6b61> in <module>
----> 1 l.coords = [(0,0), (1,1)]
AttributeError: can't set attributewhich are the default errors produced by Python for a property without a setter (the first one about CoordinateSequence is already existing on master, just added to the test for completeness). Are we OK with that test? I could also add back a setter to the |
|
@jorisvandenbossche I'm fine with that. Since we're going to have a warning period, a custom error won't be necessary. If these objects had custom setters, custom exceptions would help users differentiate between Shapely usage bugs and Shapely bugs/Python usage bugs. In 2.0, trying to modify a coordinate sequence is a Python usage bug, and a type or attribute error fits. |
|
OK, this should be good to go then! |
|
@mwtoews @jorisvandenbossche merge when you are ready. I'm happy to let the two of you be stewards of the shapely-2.0 branch while I focus on the others. |
|
I would be happy with managing the shapely-2.0 branch (as noted in #962, I would also prefer to regularly rebase it on top of master initially), but then you will need to give me commit rights ;) (I can ensure to use them wisely!) |
|
@jorisvandenbossche I'd lost track of that requirement and will make it so. |
|
Thanks! |
* keep tests for immutability
* keep tests for immutability
* keep tests for immutability
* keep tests for immutability
* keep tests for immutability
* keep tests for immutability
* keep tests for immutability
* keep tests for immutability
* keep tests for immutability
* keep tests for immutability
* keep tests for immutability
I branched off from master, so the shapely-2.0 branch first needs to be reset to master state for this to work (the actual commit is only the last one)