Closed EarlMilktea closed 1 month ago
Attention: Patch coverage is 92.93478%
with 13 lines
in your changes missing coverage. Please review.
Project coverage is 77.12%. Comparing base (
4842f68
) to head (65eb69a
). Report is 1 commits behind head on master.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
I am still unsure what kind of guarantees we are looking for.
If the user really wants to break things, they can still do so in multiple ways. For instance, even if v.flags.writeable = True
fails, v.base.flags.writeable = True
succeeds. Or, even more simply, replacing CLIFFORD
with a deep copy of itself will unlock all the read-only flags.
graphix._db.CLIFFORD = copy.deepcopy(graphix._db.CLIFFORD)
Moreover, we have no static guarantee that _lock
is used as intended (there is no type annotation in python to prevent aliasing).
On the other hand, creating read-only views is less complex and should be sufficient to prevent accidental mutations. It also seems to strike the right balance in terms of conveying our intention for safety.
v.base.flags.writeable = True
I didn't know that. Thanks!
Now that I'm aware of this hidden caveat, I'm not eager to ensure safety in _lock
as before, but also feel that it is not so bad as to be corrected immediately.
There are many other issues that need to be fixed.
@shinich1
Updated the changelog.
@thierry-martinez
I feel most of the comments are resolved/negotiated. Do you approve this PR?
Description of the change:
Get rid of
dict
fromKrausChannel
.