Update update_transformers validation
#563
Conversation
rdt/hyper_transformer.py
Outdated
| incompatible_sdtypes.append(column_name) | ||
| raise InvalidSdtypeForTransformerError( | ||
| f"Column '{column_name}' is a {current_sdtype} column, which is " | ||
| f"incompatible with the '{type(transformer).__name__}' transformer." |
There was a problem hiding this comment.
🤔 Should we add a get_name method to the BaseTransformer and use that whenever we want to get the __name__ ?
| hypertransformer.detect_initial_config(input_data) | ||
| hypertransformer.update_transformers(field_transformers) | ||
| try: | ||
| hypertransformer.update_transformers(field_transformers) | ||
| except InvalidSdtypeForTransformerError: | ||
| pass |
There was a problem hiding this comment.
I think instead we could use the set_config method and avoid having this error get raised altogether
There was a problem hiding this comment.
It will still run into an error, since we have a version of it validating set_config as well:
Lines 240 to 244 in 1c2c821
There was a problem hiding this comment.
When we set the config can't we force the sdtype to match?
rdt/errors.py
Outdated
| @@ -7,3 +7,7 @@ class NotFittedError(Exception): | |||
|
|
|||
| class Error(Exception): | |||
| """Error to raise when ``HyperTransformer`` produces a controlled error message.""" | |||
|
|
|||
|
|
|||
| class InvalidSdtypeForTransformerError(Exception): | |||
There was a problem hiding this comment.
I think we should change this error name to match the ones we created in the doc. You won't need to catch this error anymore in the integration tests if you do the other change
rdt/errors.py
Outdated
| @@ -9,5 +9,5 @@ class Error(Exception): | |||
| """Error to raise when ``HyperTransformer`` produces a controlled error message.""" | |||
|
|
|||
|
|
|||
| class InvalidSdtypeForTransformerError(Exception): | |||
| """Error to raise when the sdtype is not supported by the transformer.""" | |||
| class SynthesizerInputError(Exception): | |||
There was a problem hiding this comment.
This should be TransformerInputError. The synthesizer prefixes will only be for SDV
| hypertransformer.detect_initial_config(input_data) | ||
| hypertransformer.update_transformers(field_transformers) | ||
| try: | ||
| hypertransformer.update_transformers(field_transformers) | ||
| except InvalidSdtypeForTransformerError: | ||
| pass |
There was a problem hiding this comment.
When we set the config can't we force the sdtype to match?
Codecov ReportBase: 100.00% // Head: 100.00% // No change to project coverage 👍
Additional details and impacted files@@ Coverage Diff @@
## master #563 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 16 16
Lines 1523 1524 +1
=========================================
+ Hits 1523 1524 +1
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| 'sdtypes': sdtypes, | ||
| 'transformers': field_transformers | ||
| } | ||
| hypertransformer.detect_initial_config(input_data) |
There was a problem hiding this comment.
you can just delete this line now. You only need to detect if you don't set it
Update
update_transformersso an error is raised when the sdtype is not supported by the transformer, instead of raising an error.