Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimise plot lines #234

Merged
merged 9 commits into from
Jan 9, 2025
Merged

Optimise plot lines #234

merged 9 commits into from
Jan 9, 2025

Conversation

Estasie
Copy link
Contributor

@Estasie Estasie commented Dec 24, 2024

This PR refactors the plotLinesPlugin to improve performance.
Key improvements:

  • Replaced deepIsEqual checks with simple id-based comparisons for plot lines.
  • Switched from array-based storage to a Map, reducing the complexity of operations like addition, removal, and updates to O(1) where applicable.

@astandrik astandrik requested a review from Copilot December 24, 2024 12:01

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

Comments suppressed due to low confidence (5)

src/YagrCore/plugins/plotLines/plotLines.ts:19

  • The comment contains a grammatical error. It should be corrected to: /** This code handles unexpected Infinities in values */
/** This weird code should handles unexpected Inifinities in values */

src/YagrCore/plugins/plotLines/plotLines.ts:166

  • Ensure that the new method addPlotLines is covered by tests.
function addPlotLines(additionalPlotLines: PlotLineConfig[]) {

src/YagrCore/plugins/plotLines/plotLines.ts:176

  • Ensure that the new method removePlotLines is covered by tests.
function removePlotLines(plotLinesToRemove: PlotLineConfig[], scale?: string | undefined) {

src/YagrCore/plugins/plotLines/plotLines.ts:186

  • Ensure that the new method updatePlotLines is covered by tests.
function updatePlotLines(newPlotLines?: PlotLineConfig[] | undefined, scale?: string | undefined) {

src/YagrCore/plugins/plotLines/plotLines.ts:207

  • Ensure that the new method clearPlotLines is covered by tests.
function clearPlotLines(scale?: string) {
@Estasie Estasie requested review from zefirka and catakot December 26, 2024 09:28
@Estasie Estasie force-pushed the optimise-plot-lines branch from e3d8b2e to e80c0a5 Compare December 26, 2024 12:38
@Estasie Estasie merged commit 9439512 into main Jan 9, 2025
2 checks passed
@Estasie Estasie deleted the optimise-plot-lines branch January 9, 2025 10:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants