Allow Paths to be marked as readonly #2010
Conversation
…eadonly, and deepcopy those Paths in tests that need to modify the path.
| self._update_values() | ||
|
|
||
| @property | ||
| def simplify_threshold(self): |
There was a problem hiding this comment.
I'm not sure this property (and some of the others) really need to exist. The original attribute access was perfectly satisfactory - if you changed something that didn't make sense then you would expect it to blow up...
I guess what I'm saying is that I'm not really a big fan of protecting values in this way - if a user wants to fiddle with them, then they should be able to.
There was a problem hiding this comment.
The nice thing about this approach is that when the vertices are updated, these values are updated as well, so they should always be correct. I think that's a good thing.
There was a problem hiding this comment.
I think that's a good thing.
I agree, but there are always corner cases 😉
path.vertices[10:50] += 10
I'm happy with the read only protection for coding mistakes (such as mine) - but this is python: I don't think we need to worry about protecting the whole state of Path instances.
Either way, it's much of a muchness - and more important than whether it is a property or an actual attribute, I think there should be a oneline docstring for all of these.
There was a problem hiding this comment.
I think I'll leave this as properties -- I think in most cases this is preferred.
Agreed about the docstrings.
I realise we can't protect much, but I think this was a particularly surprising one, mainly because I think users are surprised that Path.unit_circle() is a singleton. I could have also resolved this by having those return a copy, but that breaks some optimizations in markers and elsewhere where it tries to save the same path only once wherever possible.
|
Ah -- it seems the |
Allow Paths to be marked as readonly
This PR is just to prevent the user from modifying the singleton Paths (unit_circle, unit_rectangle etc) which then cause nasty side effects for anything else that tries to use that Path later (such as the Marker infrastructure).
Issue #2003 recently "rediscovered" this bug, but we've seen it before.
The actual domain of this API change is fairly small, as most Paths created will still be mutable -- this only affects code that wants to modify one of the singleton Paths.