[GSK-2346] Avoid copying whole dataset when doing slicing

[Hartorn]

Description

DataProcessor was doing a copy every time, event in the case of slice.
The issue is the slice was actually referencing the whole dataset, so the whole dataset was kept in memory.
Now, we only reference the existing one.

Testing on another large dataset (76020, 371), was crashing before, working now.

Related Issue

Type of Change

• 📚 Examples / docs / tutorials / dependencies update
• 🔧 Bug fix (non-breaking change which fixes an issue)
• 🥂 Improvement (non-breaking change which improves an existing feature)
• 🚀 New feature (non-breaking change which adds functionality)
• 💥 Breaking change (fix or feature that would cause existing functionality to change)
• 🔐 Security fix

Checklist

• I've read the CODE_OF_CONDUCT.md document.
• I've read the CONTRIBUTING.md guide.
• I've updated the code style using make codestyle.
• I've written tests for all new methods and classes that I created.
• I've written the docstring in Google format for all the methods and classes that I used.

[linear]

GSK-2346 Tabular scan execution on wide dataset kills customer's VM

[kevinmessiaen]

Fixes the issue, However we can get ride of the copy flag altogether since doing a copy change nothing in the behaviour of the code expect loading duplicated data in the ram.

PS: it is updated in case of empty pipeline (but nothing breaking):

• Line 110: Reset the pipeline (not a big deal)
• Line 114: Recompute the column_meta -> Do nothing but takes time

[kevinmessiaen]

After reading the code, it seems that doing a copy of dataset was unnecessary since ds instance is only queried and never updated.

Suggested change

	def apply(self, dataset: "Dataset", apply_only_last=False, get_mask: bool = False, copy: bool = True):
	if copy:
	ds = dataset.copy()
	else:
	ds = dataset
	def apply(self, dataset: "Dataset", apply_only_last=False, get_mask: bool = False):
	ds = dataset

[Hartorn]

Not true, making a copy may still be necessary.
For example, in the TextTransformation, code is directly modifying the dataframe, and so copy is still needed.

To remove this copy, we need to go through every slicing function and transformation, and I don't want this pr to take too long.

[mattbit]

Makes sense!

[kevinmessiaen]

Ah my bad, you're right, I overlooked this part :/

[mattbit]

@Hartorn I would be careful because I remember having to introduce this copy operation because of some bug b092277.
I can't find the PR associated to that commit right now but I would check

[Hartorn]

@Hartorn I would be careful because I remember having to introduce this copy operation because of some bug b092277. I can't find the PR associated to that commit right now but I would check

atm TextTransformation modify dataframe directly, so I don't want to change this beahaviour either (except when slicing)

[mattbit]

At some point we'll need to improve this, but the current fix is ok for me.

[sonarqubecloud]

Kudos, SonarCloud Quality Gate passed!   

0 Bugs
0 Vulnerabilities
0 Security Hotspots
1 Code Smell

100.0% Coverage
0.0% Duplication