Add utility methods #965
Add utility methods #965
Conversation
fealho
force-pushed
the
issue-948-add-utilities
branch
from
860ac28 to
4882d0f
Compare
Codecov Report
@@ Coverage Diff @@
## V1.0.0.dev #965 +/- ##
==============================================
+ Coverage 74.51% 74.64% +0.12%
==============================================
Files 42 42
Lines 3547 3577 +30
==============================================
+ Hits 2643 2670 +27
- Misses 904 907 +3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
| """ | ||
| return { | ||
| 'metadata': copy.deepcopy(self.metadata.to_dict()), | ||
| 'constraints_to_reverse': copy.deepcopy(self._constraints_to_reverse), |
There was a problem hiding this comment.
This implementation assumes constraints_to_reverse is a list of constraint_dicts. If we decide not to do so, this needs to be revised.
There was a problem hiding this comment.
_constraints_to_reverse will actually be a list of constraint instances. When this class gets initialized it creates all the constraint instances from the metadata and then adds specific ones to this list when needed
| dict: | ||
| Dict representation of this DataProcessor. | ||
| """ | ||
| return { |
There was a problem hiding this comment.
I'm only passing metadata, constraints_to_reverse, model_kwargs because:
_constraintsare derived frommetadataduring the init (so they are not needed)._transformers_by_sdtypecannot be fully transformed into dict, since RDT transformers don't have to_dict/from_dict methods.
| } | ||
|
|
||
| @classmethod | ||
| def from_dict(cls, metadata_dict, learn_rounding_scheme=True, enforce_min_max_values=True): |
There was a problem hiding this comment.
Since to_dict doesn't provide _transformers_by_sdtype, we have to pass learn_rounding_scheme and enforce_min_max_values to derive it from scratch.
| """ | ||
| return { | ||
| 'metadata': copy.deepcopy(self.metadata.to_dict()), | ||
| 'constraints_to_reverse': copy.deepcopy(self._constraints_to_reverse), |
There was a problem hiding this comment.
_constraints_to_reverse will actually be a list of constraint instances. When this class gets initialized it creates all the constraint instances from the metadata and then adds specific ones to this list when needed
| enforce_min_max_values=enforce_min_max_values, | ||
| model_kwargs=metadata_dict.get('model_kwargs') | ||
| ) | ||
| instance._constraints_to_reverse = metadata_dict.get('constraints_to_reverse', []) |
There was a problem hiding this comment.
this will have to change as well
| @@ -97,9 +97,10 @@ def to_dict(self): | |||
| dict: | |||
| Dict representation of this DataProcessor. | |||
| """ | |||
| constraints_to_reverse = [cnt.to_dict() for cnt in self._constraints_to_reverse] | |||
There was a problem hiding this comment.
minor: does it not fit to do constraint instead of cnt. In general I think we should try to use full descriptive names
* Add tests + methods * Update to_json/from_json + test cases * Address feedback
* Add tests + methods * Update to_json/from_json + test cases * Address feedback
* Add tests + methods * Update to_json/from_json + test cases * Address feedback
* Add tests + methods * Update to_json/from_json + test cases * Address feedback
* Add tests + methods * Update to_json/from_json + test cases * Address feedback
Resolve #948.