Skip to content

Conversation

@kevinmessiaen
Copy link
Member

Description

Following code wasn't working as expected:

@slicing_function(name="slice cell level", cell_level=True)
def filter_cell_level_by(amount: int, min_value: int) -> bool:
    return amount >= min_value

df = pd.DataFrame({"quantity": [1, 2, 3, 5, 7, 11, 13]})
dataset = Dataset(df, cat_columns=[])

min_five = filter_cell_level_by(min_value=5)
min_six = filter_cell_level_by(min_value=6)

dataset_greater_equals_five = dataset.slice(min_five, column_name="quantity")
dataset_greater_equals_six = dataset.slice(min_six, column_name="quantity")

assert list(dataset_greater_equals_five.df["quantity"]) == [5, 7, 11, 13]
assert list(dataset_greater_equals_six.df["quantity"]) == [7, 11, 13]

Expected behaviour

  • dataset_greater_equals_five contains values where quantity >= 5
  • dataset_greater_equals_six contains values where quantity >= 6

Actual behaviour

dataset_greater_equals_five and dataset_greater_equals_six both contain values where quantity >= 6

This behaviour happens due to the @slicing_function decorator generating a "singleton' of SlicingFunction. Same issue is seen with the transformation functions.

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

@kevinmessiaen kevinmessiaen requested a review from henchaves July 15, 2024 06:59
@kevinmessiaen kevinmessiaen enabled auto-merge July 16, 2024 08:14
@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
24.0% Duplication on New Code (required ≤ 3%)

See analysis details on SonarCloud

@kevinmessiaen kevinmessiaen merged commit d8cd08d into main Jul 16, 2024
@kevinmessiaen kevinmessiaen deleted the fix/slicing-function-params branch July 16, 2024 08:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants