Correct id-based keying of scatter plot pixmap cache#1564
Correct id-based keying of scatter plot pixmap cache#1564j9ac9k merged 3 commits intopyqtgraph:masterfrom
Conversation
Note: naively using the id function results in non-unique keys. Alternatively, we could serialize the Qt objects and use these in the key. This would provide protection from the user mutating these later, but at a cost to performance.
|
hey @lidstrom83 good to see you back here. Thanks for this update. @danielhrisca would you mind benchmarking this branch in your tool? I'd like to see how it compares. |
|
Hey @j9ac9k, do you have any preference for the performance vs. safety trade-off between the id vs. serialization cache key approaches? |
I do not, primarily because I don't have a good way to profile so it's hard to evaluate and make a fair comparison. In terms of specific to this PR, my standard is simple, does it improve on the current implementation? yes (and it works); then merge. I don't want to hold up changes on ScatterPlotItem based on what the ideal implementation would be. |
|
|
That's absolutely great! EDIT just to verify, this is an improvement over 0.11.0 as well? |
|
@lidstrom83 you planning any other changes on this PR or is this guy ready to merge? |
|
|
|
No, this PR is done now. I'll dig further to get to the bottom of the rest of @danielhrisca's slowdown. |
|
Sounds good, Merging. Thanks @lidstrom83 @danielhrisca thanks for the benchmark info too! |
Note: naively using the id function results in non-unique keys.
Alternatively, we could serialize the Qt objects and use these in the key. This would provide protection from the user mutating these later, but at a cost to performance.