extend transformers to return empty lists when no variables found#940
Merged
Conversation
* add return_empty attr to base_numerical * Apply suggestion from @solegalli * Apply suggestion from @solegalli * code-style --------- Co-authored-by: Soledad Galli <solegalli@protonmail.com>
* add: return_empty docstring * chore: minor fix for tests * push str to the next line * expand substitute * minor fix for flake8 --------- Co-authored-by: Soledad Galli <solegalli@protonmail.com>
* add return_empty on creation * typo * adding _check_return_empty_is_bool
* adding return_empty attr * codestyle * more codestyle
* add: return_empty in imputation * minor-fix: flake8 style
* clearn deprecation warnings * fix warnings in pipeline tests * remove more warnings in tests * more changes * revert changes
#948) * add return_empty to transformation, scaling, outliers, datetime, timeseries and wrapper transformers Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * remove missplaced tests --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* fix return_empty exposure in discretisers and add tests for existing return_empty transformers EqualFrequencyDiscretiser, EqualWidthDiscretiser and GeometricWidthDiscretiser advertised return_empty in their docstrings but did not accept it in __init__, so users could not enable it. ArbitraryDiscretiser selects variables from binning_dict and does not support the parameter, so the unused docstring wiring is removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * remove missplaced tests --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
* add return_empty tests via shared estimator check, plus family-specific tests Adds check_return_empty to fit_functionality_checks.py and wires it into check_feature_engine_estimator, so it runs automatically for every transformer that already goes through that shared pipeline (transformation, discretisation, encoding, imputation, outliers, most of creation, forecasting). For transformers not fed through check_feature_engine_estimator, tests are added directly in their existing per-class test files, following the file's established conventions: DecisionTreeFeatures, DatetimeSubtraction, MeanNormalizationScaler and SklearnTransformerWrapper. DatetimeOrdinal needs a clone-free test, since its __init__ never stores self.start_date, which breaks sklearn's get_params()/clone() for this class independently of return_empty. DecisionTreeEncoder's return_empty=True path is documented as currently raising ValueError (a real bug: it builds a nested OrdinalEncoder(variables=[]) whose constructor rejects the explicit empty list), rather than silently asserting the intended contract. AddMissingIndicator, DropMissingData and RandomSampleImputer (variables="all") and SklearnTransformerWrapper wrapping an "all-types" transformer are not testable this way: find_all_variables only raises on a zero-column dataframe, which check_X rejects before variable selection ever runs. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * remove unnecessary filter * remove return_empty from shared base classes where subclasses don't need it BaseCreation, BaseDiscretiser and CategoricalInitMixinNA each set self.return_empty unconditionally, even though some of their subclasses never read it: MathFeatures and RelativeFeatures (variables is mandatory, never None), and ArbitraryDiscretiser (fit() is fully overridden, selecting variables from binning_dict instead of find_numerical_variables). Those classes ended up with a dead return_empty attribute that isn't a real, settable constructor parameter (hasattr is True, get_params() is False), which also meant the test gate had to use the less obvious get_params() check instead of the hasattr() pattern used by every other check. return_empty is removed from BaseCreation and BaseDiscretiser; the subclasses that genuinely need it (DatetimeSubtraction; EqualFrequencyDiscretiser, EqualWidthDiscretiser, GeometricWidthDiscretiser) now validate and store it directly, the same way DecisionTreeDiscretiser already did. CategoricalInitMixinNA is different: MatchCategories (feature_engine.preprocessing) was the only user not exposing return_empty, but it does exercise the return_empty-consuming code path (auto-detects categorical variables via the same shared _check_or_select_variables used by its sibling encoders), so removing the leaked attribute broke it outright. Rather than special-case it to a hardcoded False, return_empty is added as a proper, documented parameter to MatchCategories, matching OrdinalEncoder, RareLabelEncoder, CountFrequencyEncoder and MeanEncoder. The shared test gate in check_feature_engine_estimator now uses hasattr(estimator, "return_empty"), consistent with the other init-parameter checks (cv, missing_values, drop_original), instead of the get_params()-based gate needed to route around the leaks. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * replace try/except probing in check_return_empty with tag-based lookup Instead of trying each of 3 candidate dataframes against the transformer and catching whichever one raises TypeError, look up the right "no variables of this type" dataframe directly from the transformer's own `variables` tag (numerical/categorical/datetime), the same tag the other variable-selection checks already use. Two transformers don't carry a usable tag for unrelated reasons: LogTransformer's _more_tags() doesn't set "variables" at all (a pre-existing gap), and DatetimeSubtraction inherits BaseCreation's "skip" tag (meant to opt out of the *generic* variable-assignment checks, not a statement about its variable type). Both are handled with a small, explicit, named override instead of silently falling back to trial and error. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * final changes to tests * tities test and fixes transform tests * expands docstring test * fix and expand test for datetime ordinal * remove comment * fix test to sklearn wrapper and updates implementation * remove comment * fix code style * add error text matching to tests --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.