PR: Implement support for better ndarray copy management in colour.continuous.Signal class.
#1151
Conversation
…earns1988` definition.
| with ndarray_copy_enable(False): | ||
| self.assertEqual(id(ndarray_copy(a)), id(a)) | ||
|
|
||
|
|
There was a problem hiding this comment.
We should test the other decorating modes as well. This leaves the function decorator untested.
There was a problem hiding this comment.
| @@ -1788,6 +1792,142 @@ def from_range_int( | |||
| return a | |||
|
|
|||
|
|
|||
| _NDARRAY_COPY_ENABLED: bool = True | |||
There was a problem hiding this comment.
I wonder if there is something we should do (long term, not this PR) to manage the number of globals Colour creates. A lot of the caches are also globals. Users might not want to see these globals if they are trying to debug client applications, or at least it could be clearer that they are colour-"globals"
There was a problem hiding this comment.
This is a good point yeah, it should be easy to do!
colour/continuous/signal.py
Outdated
| @@ -515,6 +517,7 @@ def extrapolator_kwargs(self, value: dict): | |||
| self._function = None # Invalidate the underlying continuous function. | |||
|
|
|||
| @property | |||
| @ndarray_copy_enable(False) | |||
There was a problem hiding this comment.
It's not clear to me why these decorators are littered around... I can't see where they are actually affecting any behavior. It looks like it is a promise that this function is copy-less safe (no-side effects) but that guarantee or promise didn't exist before and doesn't appear to be adding value to the function signature here... Are we going to end up with @nd_copy_enable(False) functions everywhere?
Minor concern about long term code litter.
There was a problem hiding this comment.
This is a good concern, and one would certainly have to know what is going on here, i.e. that it disable the domain and range getter copies. We could add some comments to make it understandable. I don't think there is a need to have the decorators anywhere else than where they are.
An alternative is to have getters that are without copy, e.g. domain_no_copy or alike.
There was a problem hiding this comment.
I just did a small pass using directly the protected attributes thus removed most of the decorated calls.
…tinuous.Signal` class.
KelSolaar
force-pushed
the
feature/copyless
branch
from
27f6f4b
to
79bc5ba
Compare
Summary
This PR implements support for better
ndarraycopy management incolour.continuous.Signalclass enabling most internal operations to be performed without a copy of the data and thus enabling better performance.The newly added
colour.utilities.ndarray_copy_enablecontext manager / decorator should also be useful for other areas touched by #1120. I purposely left them untouched in this particular PR.Preflight
Code Style and Quality
colour,colour.models.Documentation