Add more functionality to the Config class #467
Conversation
Codecov Report
@@ Coverage Diff @@
## v1.0.0.dev #467 +/- ##
============================================
Coverage 100.00% 100.00%
============================================
Files 14 14
Lines 1159 1180 +21
============================================
+ Hits 1159 1180 +21
Continue to review full report at Codecov.
|
| 'sdtypes': self.field_sdtypes, | ||
| 'transformers': self.field_transformers | ||
| }) | ||
| return self.config |
There was a problem hiding this comment.
One question is whether or not we should expose the actual config class to the user. Maybe we just return the dict as it used to be to them, but when they call set_config we create the special class.
There was a problem hiding this comment.
I think this should return a custom version of the dict (what the config class used to be) that prints nicely. I don't think we want to expose this whole class to them since it might be confusing
rdt/hyper_transformer.py
Outdated
| if sdtype in self._default_sdtype_transformers: | ||
| self.field_transformers[field] = self._default_sdtype_transformers[sdtype] | ||
| default_transformer = self._default_sdtype_transformers[sdtype] | ||
| self.config['field_transformers'][field] = default_transformer |
There was a problem hiding this comment.
I was thinking of adding methods to let the HyperTransformer interact with the Config instead of doing it directly. That way people don't accidently forget to update the _provided dicts if necessary. So maybe an add_transformer(provided) method that adds it to config.field_transformers and if provided is True, then also config._provided_transformers. Same for field_sdtypes
rdt/hyper_transformer.py
Outdated
| self['field_transformers'].update(config['transformers']) | ||
|
|
||
| def __init__(self, config=None): | ||
| super().__init__() |
There was a problem hiding this comment.
The class probably no longer needs to inherit from dict. It could take a dict in the __init__ and set things up accordingly. This would discourage people from using the class like a dict and encourage them to use the setter methods we create instead
pvk-developer
force-pushed
the
issue_457_add_more_functionality_to_config_class
branch
from
5ce4474 to
33200f9
Compare
| column_name_to_transformer(dict): | ||
| Dict mapping column names to transformers to be used for that column. | ||
| provided_transformer(bool): | ||
| Wether or not to add to `self._provided_field_transformers`. |
| self.field_transformers[field] = transformer | ||
|
|
||
| def update_transformers(self, column_name_to_transformer, provided_transformer=False): | ||
| """Update `self.field_transformers`. |
There was a problem hiding this comment.
I think we agreed to do double ` characters instead of just one.
| self.set_config(config) | ||
|
|
||
| def reset(self): | ||
| """Reset the `field_sdtypes` and `field_transformers`.""" |
| 'sdtypes': self.field_sdtypes, | ||
| 'transformers': self.field_transformers | ||
| }) | ||
| return self.config |
There was a problem hiding this comment.
I think this should return a custom version of the dict (what the config class used to be) that prints nicely. I don't think we want to expose this whole class to them since it might be confusing
|
We may rework this in the future. |
amontanez24
deleted the
issue_457_add_more_functionality_to_config_class
branch
Resolve #457