deepchecks
deepchecks copied to clipboard
[draft] fix logic
@nirhutnik
few thoughts/suggestions
Point 1.
IMO, we should create context instances in the same way and with the same defaults as how base checks do that. We need to separate that logic from base checks run methods into separate functions that will be reused everywhere, in one of the ways:
# == Separate base check methods ==
class SingleDatasetCheck(...):
def prepare_context(self, ...):
return ...
@merge_args(SingleDatasetCheck.prepare_context)
def run(self, *args, **kwargs):
context = self.prepare_context(*args, **kwargs)
...
# or
# == Context factory methods ==
class Context:
@classmethod
def for_single_dataset_check(cls, ...):
...
class SingleDatasetCheck(...):
@merge_args(Context.for_single_dataset_check)
def run(self, *args, **kwargs):
context = Context.for_single_dataset_check(*args, **kwargs)
...
Point 2.
SingleDatasetCheckFixMixin
and TrainTestCheckFixMixin
are not needed at all, at least currently, because you can just do isinstance(check, (SingleDatasetCheck, TrainTestCheck))
Point 3.
I would suggest moving the 'fix' logic into a separate sub-package in order to not overload check classes with too much logic, something like
# fix sub-package
class MixedNullsFix:
def __init__(self, some_arg):
self.some_arg = some_arg
def fix(self, context):
...
# check module
class MixedNullCheck(...):
@merge_args(...)
def fix(self, *args, some_arg: Any = None, **kwargs):
from ... import MixedNullFix
context = self.prepare_context(*args, **kwargs)
fix = MixedNullFix(some_args)
return fix.fix(context)
but to be honest, not sure about it yet, most of the fix logic currently looks pretty simple