[FIX] Only deepcopy the .attributes for the outermost Table transformation #6849
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.
Issue
#6189 introduced deep-copying of the dictionary
Table.attributes
at table transformations. Because table transformations often happen withing table transformation, this can lead to wasting computation and memory. An example of the bug that can happen is Quasars/orange-spectroscopy/pull/726.For illustration, here is a small example:
The memory use/time graphs below (first master, then this branch) show big differences (~30 seconds vs 1 second) that all stem from the wasteful use of deepcopy:
Description of changes
The code now only copies
.attributes
only for the outermost transformations.And the thing that should be discussed.This PRThe previous version of the PR would introduce problems if deeper internal transformations wanted to readtable.attributes
. I intentionally left it so just to see if there are any problems. I could add support for them by just referencing that dict for non internal transformations, but if any were naughty and actually modifiedtable.attributes
, we'd have problems.I discussed above with myself and noticed that there are no mechanisms for compute_values to set output
table.attributes
and that even before this PR, if any compute_value was particularly naughty, there were problems. The following test fails on master.The last commit therefore ensures that
.attributes
are always set, but yes, as before, they should not be modified within thecompute_value
.Includes