-
Notifications
You must be signed in to change notification settings - Fork 26
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.
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
Validate set_config
#494
Validate set_config
#494
Conversation
rdt/hyper_transformer.py
Outdated
|
||
sdtypes = config['sdtypes'] | ||
transformers = config['transformers'] | ||
if sdtypes.keys() != transformers.keys(): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know if they need to be in the same order. Can we just check that they contain the same elements?
Codecov Report
@@ Coverage Diff @@
## v1.0.0.dev #494 +/- ##
============================================
Coverage 100.00% 100.00%
============================================
Files 14 14
Lines 1239 1253 +14
============================================
+ Hits 1239 1253 +14
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small comment but otherwise looks good to me!
tests/unit/test_hyper_transformer.py
Outdated
The method should crash if ``sdytpes`` and ``transformers`` have different of columns. | ||
|
||
Input: | ||
- A config with only ``transformers``. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This isn't true for this case right?
38bd588
to
209466c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is ready!
d91c28c
to
c9ce1d0
Compare
Resolve #478.