10000 Common test to check that estimator state does not change at transform/predict/predict_proba time · Issue #7297 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

Common test to check that estimator state does not change at transform/predict/predict_proba time #7297

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.

Sign up for GitHub

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

Jump to bottom
Closed
MechCoder opened this issue Aug 30, 2016 · 7 comments
Closed

Common test to check that estimator state does not change at transform/predict/predict_proba time #7297

MechCoder opened this issue Aug 30, 2016 · 7 comments

Comments

@MechCoder
Copy link
Member
MechCoder commented Aug 30, 2016 •
edited
Loading

Description

I feel the design of #6607 could have been better thought of, if we had a common test that tests that learning happens only during fit time. We should check __dict__ after fit and after transform and their state should not change.
@amueller
Copy link
Member

Sorry, I didn't follow #6607 closely. Only in fit as opposed to what? transform in this case, I guess. We already check that nothing happens in __init__ in several ways.

If you call transform before calling fit, you usually get an NotFittedError but there are some "stateless transformers" like the HashingVectorizer and the Chi2Sampler that don't require fit.
These could be checked by comparing the dict before and after transform. Nothing should change. transform needs to not change the estimator.

Or is the question more "after fitting, make sure that transform doesn't modify the estimator"?
Again, we could copy the dict and check that it's the same? All things should be is identical. If a reference changed, we messed up.

@MechCoder
Copy link
Member Author

These could be checked by comparing the dict before and after transform.

Sure, that is what I meant (and maybe predict and predict_proba to do a sanity check). The problem in #6607 was we set the attribute indicator_features_ during transform time.

@MechCoder MechCoder changed the title Common test to check that learning happens only during fit time. Common test to check that estimator state does not change at transform/predict/predict_proba time Aug 30, 2016
@MechCoder
Copy link
Member Author

I've updated the issue description.

@kiote
Copy link
Contributor
kiote commented Sep 18, 2016

I'd like to handle this, but bear in mind I'm a total noob in the internals of sc-learn 😷

@MechCoder
Copy link
Member Author

Sure, thanks! I'm removing the Need Contributor label.

@kiote
Copy link
Contributor
kiote commented Sep 27, 2016

sorry, have a tough days at work, but hope to continue here soon

@kiote kiote mentioned this issue Oct 3, 2016
Merged
jnothman pushed a commit that referenced this issue Nov 5, 2016
@kiote @jnothman
[MRG] TST Ensure __dict__ is unmodified by predict, transform, etc (#…
02cc6f5 
…7553)

* Add test for __dict__ for estimator checks

  check that "predict", "transform", "decision_function" or
"predict_proba" methods do not change the state of __dict__ of any
estimator

  see #7297

* Add a test to test_check_estimator

  that shows that check_estimator fails on an estimator that violates this

* Fixed bug where NMF's n_iter_ attribute was set by calls to transform
amueller pushed a commit to amueller/scikit-learn that referenced this issue Nov 9, 2016
@kiote @amueller
[MRG] TST Ensure __dict__ is unmodified by predict, transform, etc (s…
20efa95 
…cikit-learn#7553)

* Add test for __dict__ for estimator checks

  check that "predict", "transform", "decision_function" or
"predict_proba" methods do not change the state of __dict__ of any
estimator

  see scikit-learn#7297

* Add a test to test_check_estimator

  that shows that check_estimator fails on an estimator that violates this

* Fixed bug where NMF's n_iter_ attribute was set by calls to transform
sergeyf pushed a commit to sergeyf/scikit-learn that referenced this issue Feb 28, 2017
@kiote @sergeyf
[MRG] TST Ensure __dict__ is unmodified by predict, transform, etc (s…
f823612 
…cikit-learn#7553)

* Add test for __dict__ for estimator checks

  check that "predict", "transform", "decision_function" or
"predict_proba" methods do not change the state of __dict__ of any
estimator

  see scikit-learn#7297

* Add a test to test_check_estimator

  that shows that check_estimator fails on an estimator that violates this

* Fixed bug where NMF's n_iter_ attribute was set by calls to transform
Sundrique pushed a commit to Sundrique/scikit-learn that referenced this issue Jun 14, 2017
@kiote @Sundrique
[MRG] TST Ensure __dict__ is unmodified by predict, transform, etc (s…
f425ef9 
…cikit-learn#7553)

* Add test for __dict__ for estimator checks

  check that "predict", "transform", "decision_function" or
"predict_proba" methods do not change the state of __dict__ of any
estimator

  see scikit-learn#7297

* Add a test to test_check_estimator

  that shows that check_estimator fails on an estimator that violates this

* Fixed bug where NMF's n_iter_ attribute was set by calls to transform
NelleV pushed a commit to NelleV/scikit-learn that referenced this issue Aug 11, 2017
@kiote @NelleV
[MRG] TST Ensure __dict__ is unmodified by predict, transform, etc (s…
cff14bf 
…cikit-learn#7553)

* Add test for __dict__ for estimator checks

  check that "predict", "transform", "decision_function" or
"predict_proba" methods do not change the state of __dict__ of any
estimator

  see scikit-learn#7297

* Add a test to test_check_estimator

  that shows that check_estimator fails on an estimator that violates this

* Fixed bug where NMF's n_iter_ attribute was set by calls to transform
paulha pushed a commit to paulha/scikit-learn that referenced this issue Aug 19, 2017
@kiote @paulha
[MRG] TST Ensure __dict__ is unmodified by predict, transform, etc (s…
e3e9ce1 
…cikit-learn#7553)

* Add test for __dict__ for estimator checks

  check that "predict", "transform", "decision_function" or
"predict_proba" methods do not change the state of __dict__ of any
estimator

  see scikit-learn#7297

* Add a test to test_check_estimator

  that shows that check_estimator fails on an estimator that violates this

* Fixed bug where NMF's n_iter_ attribute was set by calls to transform
maskani-moh pushed a commit to maskani-moh/scikit-learn that referenced this issue Nov 15, 2017
@kiote @maskani-moh
[MRG] TST Ensure __dict__ is unmodified by predict, transform, etc (s…
500712d 
…cikit-learn#7553)

* Add test for __dict__ for estimator checks

  check that "predict", "transform", "decision_function" or
"predict_proba" methods do not change the state of __dict__ of any
estimator

  see scikit-learn#7297

* Add a test to test_check_estimator

  that shows that check_estimator fails on an estimator that violates this

* Fixed bug where NMF's n_iter_ attribute was set by calls to transform
@rth
Copy link
Member
rth commented Jun 21, 2019

Fixed in #7553

@rth rth closed this as completed Jun 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@kiote @amueller @rth @MechCoder
0