PropensityFeatureStandardization deepcopy fix
#35
Conversation
This file contains 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
removed calculating of feature functions on init this makes the object un-deepcopyable. the reason is that the set of feature functions store a copy of self that does not get updated on deepcopy. since the function is only called once, and it is a light function, i opted to remove the attribute entirely and just create the dict when it is needed. that way self will never be misreferenced. Add deepcopy tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
@mmdanziger:
Seems like because the
_feature_functionswere calculated based onselfin__init__and then stored in an attribute they became effectively un-deepcopy-able. What was happening was that the deepcopied version would run fit on the fresh learner but then when it came to calculated thefeature_functionsit would use the old learner which had not been fit and was stored in the_feature_functionsattribute.The root of the problem is having
_feature_functionsas an attribute at all. It's a light function and only calculated once per predict so the added value of caching is dubious and creates cache invalidation problems. This required removing the attribute and replacing it with a lookup function.