Skip to content
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

Eliminate is_fitted_ sklearn attribute #280

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

folmos-at-orange
Copy link
Member

@folmos-at-orange folmos-at-orange commented Nov 18, 2024

commit 720d30a (HEAD -> 273-eliminate-is_fitted_-sklearn-attribute, origin/273-eliminate-is_fitted_-sklearn-attribute)
Author: Felipe Olmos 92923444+folmos-at-orange@users.noreply.github.com
Date: Mon Nov 18 17:05:40 2024 +0100

Add failure tests for export estimator operations

commit af11895
Author: Felipe Olmos 92923444+folmos-at-orange@users.noreply.github.com
Date: Mon Nov 18 16:42:04 2024 +0100

Eliminate is_fitted_ sklearn attribute

@folmos-at-orange folmos-at-orange linked an issue Nov 18, 2024 that may be closed by this pull request
@folmos-at-orange folmos-at-orange self-assigned this Nov 18, 2024
@folmos-at-orange folmos-at-orange force-pushed the 273-eliminate-is_fitted_-sklearn-attribute branch 5 times, most recently from 68850a3 to d2595f1 Compare November 25, 2024 10:20
@folmos-at-orange folmos-at-orange force-pushed the 273-eliminate-is_fitted_-sklearn-attribute branch from d2595f1 to 66dc651 Compare November 28, 2024 08:19
@folmos-at-orange folmos-at-orange force-pushed the 273-eliminate-is_fitted_-sklearn-attribute branch 2 times, most recently from dc12f61 to 19e05f2 Compare December 11, 2024 07:54
@folmos-at-orange folmos-at-orange force-pushed the 273-eliminate-is_fitted_-sklearn-attribute branch from 19e05f2 to 1477896 Compare December 19, 2024 18:05
tests/test_sklearn.py Outdated Show resolved Hide resolved
Copy link
Collaborator

@popescu-v popescu-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the comments.

@folmos-at-orange folmos-at-orange force-pushed the 273-eliminate-is_fitted_-sklearn-attribute branch from 1477896 to 0c9fb9f Compare January 8, 2025 08:43
@@ -2704,3 +2706,25 @@ def test_khiops_encoder_no_output_variables_implies_not_fit(self):

# Check that the encoder is not fit
self.assertNotFit(khe)

def test_export_operations_raise_when_not_fitted(self):
"""Test that export functions raise NonFittedError exceptions when non-fitted
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/NonFittedError/NotFittedError/ (Not instead of Non)

Copy link
Collaborator

@popescu-v popescu-v left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but there remains a really small typo to fix (see remaining comment).

@folmos-at-orange folmos-at-orange force-pushed the 273-eliminate-is_fitted_-sklearn-attribute branch from 0c9fb9f to 0b1671d Compare January 8, 2025 11:24
@folmos-at-orange folmos-at-orange merged commit d093ae8 into dev Jan 8, 2025
19 checks passed
@folmos-at-orange folmos-at-orange deleted the 273-eliminate-is_fitted_-sklearn-attribute branch January 8, 2025 11:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Eliminate is_fitted_ sklearn attribute
2 participants