-
Notifications
You must be signed in to change notification settings - Fork 1
test: add pytest contract suite (57 tests) and fix two silent runtime bugs #2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,47 @@ | ||
| # Contributing to MLTools | ||
|
|
||
| Thank you for considering contributing to MLTools. This document explains how to | ||
| get set up and how contributions are reviewed. | ||
|
|
||
| ## Getting Started | ||
|
|
||
| Install the library and the test dependencies in an isolated environment: | ||
|
|
||
| ```bash | ||
| python -m venv venv | ||
| source venv/bin/activate # Windows: venv\Scripts\activate | ||
| pip install -r requirements.txt | ||
| pip install -r requirements-test.txt | ||
| ``` | ||
|
|
||
| ## Running Tests | ||
|
|
||
| All behavior contracts are verified by the `pytest` suite in `tests/`. Run the | ||
| full suite before opening a pull request: | ||
|
|
||
| ```bash | ||
| pytest tests/ | ||
| ``` | ||
|
|
||
| The legacy smoke script `test_mltools.py` is also kept in the repository and | ||
| should continue to pass. | ||
|
|
||
| ## Adding Tests | ||
|
|
||
| New public behavior must be covered by contract tests under `tests/`. Tests | ||
| should be organized by module (for example `test_preprocessing.py`) and grouped | ||
| in classes describing the API surface they verify. Each test should assert the | ||
| behavioral contract (input validation, edge cases, error paths, output shape), | ||
| not internal implementation details. | ||
|
|
||
| ## Pull Requests | ||
|
|
||
| Pull requests are reviewed against the following checklist: | ||
|
|
||
| 1. Does the change fix a real bug or add verifiable value? | ||
| 2. Are new behaviors covered by tests that pass locally? | ||
| 3. Does the existing smoke script (`test_mltools.py`) still pass? | ||
| 4. Are there no secrets, credentials, or unrelated changes in the diff? | ||
|
|
||
| Describe the problem, the reason, the solution, and the tests in the pull | ||
| request description so reviewers can assess impact quickly. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| pytest>=8.0 |
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,192 @@ | ||||||||||||||||||||||||||||||||||||||||||||||
| """Contract tests for ModelEvaluator, DataExplorer, and utils helpers.""" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| import os | ||||||||||||||||||||||||||||||||||||||||||||||
| import pandas as pd | ||||||||||||||||||||||||||||||||||||||||||||||
| import numpy as np | ||||||||||||||||||||||||||||||||||||||||||||||
| import pytest | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| from mltools import Config, DataExplorer | ||||||||||||||||||||||||||||||||||||||||||||||
| from mltools.evaluation import ModelEvaluator | ||||||||||||||||||||||||||||||||||||||||||||||
| from mltools.utils import save_model, load_model, optimize_memory, detect_feature_types | ||||||||||||||||||||||||||||||||||||||||||||||
| from mltools.utils.helpers import handle_missing_values | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # --------------------------------------------------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||
| # ModelEvaluator | ||||||||||||||||||||||||||||||||||||||||||||||
| # --------------------------------------------------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| class TestModelEvaluator: | ||||||||||||||||||||||||||||||||||||||||||||||
| def test_classification_metrics_keys(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| evaluator = ModelEvaluator() | ||||||||||||||||||||||||||||||||||||||||||||||
| y_true = np.array([0, 1, 1, 0, 1]) | ||||||||||||||||||||||||||||||||||||||||||||||
| y_pred = np.array([0, 1, 0, 0, 1]) | ||||||||||||||||||||||||||||||||||||||||||||||
| metrics = evaluator.evaluate_classification(y_true, y_pred) | ||||||||||||||||||||||||||||||||||||||||||||||
| for key in ("accuracy", "precision", "recall", "f1", "confusion_matrix"): | ||||||||||||||||||||||||||||||||||||||||||||||
| assert key in metrics | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_perfect_predictions_accuracy_one(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| evaluator = ModelEvaluator() | ||||||||||||||||||||||||||||||||||||||||||||||
| y = np.array([0, 1, 0, 1, 0, 1]) | ||||||||||||||||||||||||||||||||||||||||||||||
| metrics = evaluator.evaluate_classification(y, y) | ||||||||||||||||||||||||||||||||||||||||||||||
| assert metrics["accuracy"] == 1.0 | ||||||||||||||||||||||||||||||||||||||||||||||
| assert metrics["f1"] == 1.0 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_regression_metrics_keys(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| evaluator = ModelEvaluator() | ||||||||||||||||||||||||||||||||||||||||||||||
| y_true = np.array([1.0, 2.0, 3.0, 4.0]) | ||||||||||||||||||||||||||||||||||||||||||||||
| y_pred = np.array([1.1, 2.2, 2.9, 3.8]) | ||||||||||||||||||||||||||||||||||||||||||||||
| metrics = evaluator.evaluate_regression(y_true, y_pred) | ||||||||||||||||||||||||||||||||||||||||||||||
| for key in ("mse", "rmse", "mae", "r2"): | ||||||||||||||||||||||||||||||||||||||||||||||
| assert key in metrics | ||||||||||||||||||||||||||||||||||||||||||||||
| assert metrics["mse"] >= 0 | ||||||||||||||||||||||||||||||||||||||||||||||
| assert 0.0 <= metrics["r2"] <= 1.0 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_perfect_regression_predictions(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| evaluator = ModelEvaluator() | ||||||||||||||||||||||||||||||||||||||||||||||
| y = np.array([1.0, 2.0, 3.0, 4.0]) | ||||||||||||||||||||||||||||||||||||||||||||||
| metrics = evaluator.evaluate_regression(y, y) | ||||||||||||||||||||||||||||||||||||||||||||||
| assert metrics["mse"] == 0.0 | ||||||||||||||||||||||||||||||||||||||||||||||
| assert metrics["r2"] == 1.0 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_mismatched_lengths_raises(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| evaluator = ModelEvaluator() | ||||||||||||||||||||||||||||||||||||||||||||||
| with pytest.raises(ValueError): | ||||||||||||||||||||||||||||||||||||||||||||||
| evaluator.evaluate_classification(np.array([0, 1]), np.array([0, 1, 0])) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_get_results_after_evaluate(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| evaluator = ModelEvaluator() | ||||||||||||||||||||||||||||||||||||||||||||||
| y = np.array([0, 1, 1, 0]) | ||||||||||||||||||||||||||||||||||||||||||||||
| evaluator.evaluate_classification(y, y) | ||||||||||||||||||||||||||||||||||||||||||||||
| assert evaluator.get_results() is not None | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # --------------------------------------------------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||
| # DataExplorer | ||||||||||||||||||||||||||||||||||||||||||||||
| # --------------------------------------------------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| class TestDataExplorer: | ||||||||||||||||||||||||||||||||||||||||||||||
| def test_summary_statistics_columns(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| """Summary statistics must include descriptive stats plus the | ||||||||||||||||||||||||||||||||||||||||||||||
| library's missing-value profile columns.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| data = pd.DataFrame( | ||||||||||||||||||||||||||||||||||||||||||||||
| {"a": [1.0, 2.0, 3.0], "b": [4.0, 5.0, 6.0], "c": ["x", "y", "z"]} | ||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||
| explorer = DataExplorer(data) | ||||||||||||||||||||||||||||||||||||||||||||||
| summary = explorer.summary_statistics() | ||||||||||||||||||||||||||||||||||||||||||||||
| for key in ("mean", "std", "missing", "missing_pct"): | ||||||||||||||||||||||||||||||||||||||||||||||
| assert key in summary.columns | ||||||||||||||||||||||||||||||||||||||||||||||
| # the categorical column must still be profiled | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "c" in summary.columns or "c" in summary.index | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_missing_value_analysis(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| """analyze_missing_values must report each column with missing | ||||||||||||||||||||||||||||||||||||||||||||||
| values together with its percentage, as a column-oriented table.""" | ||||||||||||||||||||||||||||||||||||||||||||||
| data = pd.DataFrame({"a": [1.0, np.nan, 3.0], "b": [np.nan, np.nan, np.nan]}) | ||||||||||||||||||||||||||||||||||||||||||||||
| explorer = DataExplorer(data) | ||||||||||||||||||||||||||||||||||||||||||||||
| missing = explorer.analyze_missing_values() | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "column" in missing.columns | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "missing_percentage" in missing.columns | ||||||||||||||||||||||||||||||||||||||||||||||
| reported = set(missing["column"]) | ||||||||||||||||||||||||||||||||||||||||||||||
| assert reported == {"a", "b"}, "both columns with NaNs must be reported" | ||||||||||||||||||||||||||||||||||||||||||||||
| row_b = missing.loc[missing["column"] == "b"].iloc[0] | ||||||||||||||||||||||||||||||||||||||||||||||
| assert row_b["missing_percentage"] == 100.0 | ||||||||||||||||||||||||||||||||||||||||||||||
| assert row_b["missing_count"] == 3 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_correlation_analysis(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| rng = np.random.default_rng(42) | ||||||||||||||||||||||||||||||||||||||||||||||
| a = rng.standard_normal(50) | ||||||||||||||||||||||||||||||||||||||||||||||
| data = pd.DataFrame({"a": a, "b": a + rng.standard_normal(50) * 0.1}) | ||||||||||||||||||||||||||||||||||||||||||||||
| explorer = DataExplorer(data) | ||||||||||||||||||||||||||||||||||||||||||||||
| corr = explorer.correlation_analysis() | ||||||||||||||||||||||||||||||||||||||||||||||
| assert corr.loc["a", "b"] > 0.9 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_generate_report_keys(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| data = pd.DataFrame( | ||||||||||||||||||||||||||||||||||||||||||||||
| {"a": [1.0, 2.0, 3.0, 4.0], "b": [4.0, 5.0, 6.0, 7.0], "target": [0, 1, 0, 1]} | ||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||
| explorer = DataExplorer(data.drop(columns=["target"])) | ||||||||||||||||||||||||||||||||||||||||||||||
| report = explorer.generate_report() | ||||||||||||||||||||||||||||||||||||||||||||||
| assert isinstance(report, dict) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+103
to
+110
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Assert the report schema. This test passes when Proposed fix report = explorer.generate_report()
assert isinstance(report, dict)
+ assert {
+ "shape",
+ "summary_statistics",
+ "missing_values",
+ "correlation_matrix",
+ "feature_types",
+ "memory_usage_mb",
+ } <= report.keys()📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # --------------------------------------------------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||
| # Config | ||||||||||||||||||||||||||||||||||||||||||||||
| # --------------------------------------------------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| class TestConfig: | ||||||||||||||||||||||||||||||||||||||||||||||
| def test_save_and_load_roundtrip(self, tmp_path): | ||||||||||||||||||||||||||||||||||||||||||||||
| config = Config() | ||||||||||||||||||||||||||||||||||||||||||||||
| config.random_state = 7 | ||||||||||||||||||||||||||||||||||||||||||||||
| config.preprocessing["scale_numerical"] = "standard" | ||||||||||||||||||||||||||||||||||||||||||||||
| path = tmp_path / "config.json" | ||||||||||||||||||||||||||||||||||||||||||||||
| config.save(str(path)) | ||||||||||||||||||||||||||||||||||||||||||||||
| loaded = Config.load(str(path)) | ||||||||||||||||||||||||||||||||||||||||||||||
| assert loaded.random_state == 7 | ||||||||||||||||||||||||||||||||||||||||||||||
| assert loaded.preprocessing["scale_numerical"] == "standard" | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_defaults(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| config = Config() | ||||||||||||||||||||||||||||||||||||||||||||||
| assert config.random_state == 42 | ||||||||||||||||||||||||||||||||||||||||||||||
| assert config.preprocessing["handle_missing"] == "smart" | ||||||||||||||||||||||||||||||||||||||||||||||
| assert config.splitting["test_size"] == 0.2 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_update_scopes_to_dict(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| config = Config() | ||||||||||||||||||||||||||||||||||||||||||||||
| config.update(preprocessing={"handle_missing": "drop"}) | ||||||||||||||||||||||||||||||||||||||||||||||
| assert config.preprocessing["handle_missing"] == "drop" | ||||||||||||||||||||||||||||||||||||||||||||||
| # unrelated keys are untouched | ||||||||||||||||||||||||||||||||||||||||||||||
| assert config.random_state == 42 | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| # --------------------------------------------------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||
| # helpers: optimize_memory, detect_feature_types, save/load_model | ||||||||||||||||||||||||||||||||||||||||||||||
| # --------------------------------------------------------------------------- | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| class TestUtilsHelpers: | ||||||||||||||||||||||||||||||||||||||||||||||
| def test_detect_feature_types_classes_columns(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| data = pd.DataFrame( | ||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||
| "num": [1.0, 2.0, 3.0], | ||||||||||||||||||||||||||||||||||||||||||||||
| "cat": ["a", "b", "a"], | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||
| types = detect_feature_types(data) | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "num" in types["numerical"] | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "cat" in types["categorical"] | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_detect_feature_types_datetime_columns(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| data = pd.DataFrame( | ||||||||||||||||||||||||||||||||||||||||||||||
| {"ts": pd.to_datetime(["2024-01-01", "2024-01-02", "2024-01-03"])} | ||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||
| types = detect_feature_types(data) | ||||||||||||||||||||||||||||||||||||||||||||||
| assert "ts" in types["datetime"] | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_optimize_memory_preserves_values(self): | ||||||||||||||||||||||||||||||||||||||||||||||
| data = pd.DataFrame( | ||||||||||||||||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||||||||||||||||
| "small_int": [1, 2, 3], | ||||||||||||||||||||||||||||||||||||||||||||||
| "big_float": [1.5, 2.5, 3.5], | ||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||
| out = optimize_memory(data) | ||||||||||||||||||||||||||||||||||||||||||||||
| pd.testing.assert_frame_equal(data, out, check_dtype=False) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_save_load_model_roundtrip(self, tmp_path): | ||||||||||||||||||||||||||||||||||||||||||||||
| from sklearn.tree import DecisionTreeClassifier | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| model = DecisionTreeClassifier(random_state=42) | ||||||||||||||||||||||||||||||||||||||||||||||
| model.fit([[0], [1]], [0, 1]) | ||||||||||||||||||||||||||||||||||||||||||||||
| path = str(tmp_path / "model.joblib") | ||||||||||||||||||||||||||||||||||||||||||||||
| save_model(model, path) | ||||||||||||||||||||||||||||||||||||||||||||||
| loaded = load_model(path) | ||||||||||||||||||||||||||||||||||||||||||||||
| np.testing.assert_array_equal( | ||||||||||||||||||||||||||||||||||||||||||||||
| model.predict([[0], [1]]), loaded.predict([[0], [1]]) | ||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| def test_save_model_creates_parent_directories(self, tmp_path): | ||||||||||||||||||||||||||||||||||||||||||||||
| from sklearn.tree import DecisionTreeClassifier | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| model = DecisionTreeClassifier() | ||||||||||||||||||||||||||||||||||||||||||||||
| path = str(tmp_path / "nested" / "deep" / "model.joblib") | ||||||||||||||||||||||||||||||||||||||||||||||
| save_model(model, path) | ||||||||||||||||||||||||||||||||||||||||||||||
| assert os.path.exists(path) | ||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,95 @@ | ||
| """Contract tests for FeatureEngineer.""" | ||
|
|
||
| import pandas as pd | ||
| import numpy as np | ||
| import pytest | ||
|
|
||
| from mltools.preprocessing import FeatureEngineer | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def numeric_data(): | ||
| """Simple numeric DataFrame suitable for feature engineering.""" | ||
| rng = np.random.default_rng(42) | ||
| return pd.DataFrame( | ||
| { | ||
| "a": rng.standard_normal(50), | ||
| "b": rng.standard_normal(50), | ||
| "c": rng.standard_normal(50), | ||
| } | ||
| ) | ||
|
|
||
|
|
||
| class TestFeatureEngineer: | ||
| def test_statistical_features_shape(self, numeric_data): | ||
| """Statistical features append derived transforms (log, sqrt, square) | ||
| per numeric feature while preserving rows.""" | ||
| engineer = FeatureEngineer() | ||
| original_cols = numeric_data.shape[1] | ||
| # NOTE: create_statistical_features mutates the input frame (adds | ||
| # derived columns in place), so a fresh frame is used for each check. | ||
| out = engineer.create_statistical_features(numeric_data.copy()) | ||
| assert out.shape[0] == numeric_data.shape[0] | ||
| assert out.shape[1] > original_cols | ||
| # each numeric feature gains exactly three derived columns | ||
| assert out.shape[1] == original_cols * 4 | ||
|
|
||
| def test_interaction_features_shape(self, numeric_data): | ||
| """Interaction features add pairwise combinations of numeric | ||
| features while preserving rows.""" | ||
| engineer = FeatureEngineer() | ||
| out = engineer.create_interaction_features(numeric_data) | ||
| assert out.shape[0] == numeric_data.shape[0] | ||
| # with only three numeric features the current implementation may | ||
| # skip interaction generation; the pinned contract is simply that | ||
| # rows are preserved and no crash occurs. | ||
| assert out.shape[1] >= numeric_data.shape[1] | ||
|
Comment on lines
+41
to
+46
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win Assert generated feature output. Both tests can pass when the implementation returns the input unchanged. Assert the expected interaction columns for the three input columns. Assert that PCA generates its documented component columns and component count.
📍 Affects 1 file
🤖 Prompt for AI Agents |
||
|
|
||
| def test_polynomial_features_shape(self, numeric_data): | ||
| engineer = FeatureEngineer() | ||
| original_cols = numeric_data.shape[1] | ||
| out = engineer.create_polynomial_features(numeric_data.copy(), fit=True) | ||
| assert out.shape[0] == numeric_data.shape[0] | ||
| assert out.shape[1] > original_cols | ||
|
|
||
| def test_fit_transform_row_count_preserved(self, numeric_data): | ||
| engineer = FeatureEngineer() | ||
| out = engineer.fit_transform( | ||
| numeric_data, | ||
| enable_polynomial=True, | ||
| enable_interaction=True, | ||
| enable_statistical=True, | ||
| enable_clustering=False, | ||
| enable_pca=False, | ||
| ) | ||
| assert out.shape[0] == numeric_data.shape[0] | ||
| assert out.shape[1] > numeric_data.shape[1] | ||
|
|
||
| def test_disable_all_engineering_is_identity(self, numeric_data): | ||
| engineer = FeatureEngineer() | ||
| out = engineer.fit_transform( | ||
| numeric_data.copy(), | ||
| enable_polynomial=False, | ||
| enable_interaction=False, | ||
| enable_statistical=False, | ||
| ) | ||
| pd.testing.assert_frame_equal(numeric_data, out) | ||
|
|
||
| def test_input_not_mutated(self, numeric_data): | ||
| engineer = FeatureEngineer() | ||
| snapshot = numeric_data.copy() | ||
| engineer.fit_transform(numeric_data, enable_polynomial=True) | ||
| pd.testing.assert_frame_equal(numeric_data, snapshot) | ||
|
|
||
| def test_pca_preserves_row_count(self, numeric_data): | ||
| """PCA features must keep the same number of rows as the input.""" | ||
| engineer = FeatureEngineer() | ||
| out = engineer.create_pca_features(numeric_data, fit=True) | ||
| assert out.shape[0] == numeric_data.shape[0] | ||
| assert out.shape[1] >= 1 | ||
|
|
||
| def test_single_column_statistical_features(self): | ||
| """A single numeric column must still produce statistical features.""" | ||
| df = pd.DataFrame({"a": [1.0, 2.0, 3.0, 4.0, 5.0]}) | ||
| out = FeatureEngineer().create_statistical_features(df) | ||
| assert out.shape[1] > 1 | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Provide a Windows activation command.
Line 12 shows a Windows path, but the command still uses the Unix-only
sourcesyntax. Windows contributors cannot use this command as written.Proposed fix
📝 Committable suggestion
🤖 Prompt for AI Agents