Skip to content

Conversation

@ddobie
Copy link
Contributor

@ddobie ddobie commented May 20, 2025

Part 3 of pulling apart #834 into manageable chunks

@ddobie ddobie changed the base branch from dev to v2.0 May 20, 2025 08:20
@ddobie ddobie requested a review from mauch May 21, 2025 00:49
Copy link
Contributor

@mauch mauch left a comment

Choose a reason for hiding this comment

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

Just a minor comment about moving a cheap compute step inside the conditional block where it's needed. Not a vital change though.

# Explode out the img_diff column.
extr_df = extr_df.explode("img_diff").reset_index()
total_to_extract = extr_df.shape[0]
total_to_extract = extr_df.shape[0].compute()
Copy link
Contributor

Choose a reason for hiding this comment

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

You can potentially move this line back inside the if add_mode: condition below since total_to_extract is never used outside the condition. Its not vital though - since that compute() should be pretty cheap.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah that is my mistake - there should be a logging statement directly after that which I forgot to copy across

@ddobie ddobie merged commit 3b0558e into v2.0 May 21, 2025
4 checks passed
@ddobie ddobie deleted the fix-memory-leaks branch May 21, 2025 04:19
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.

3 participants