Align default transformers between SDV and RDT #1506
Conversation
Codecov ReportPatch coverage:
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. Additional details and impacted files@@ Coverage Diff @@
## master #1506 +/- ##
=======================================
Coverage 96.28% 96.28%
=======================================
Files 49 49
Lines 3931 3932 +1
=======================================
+ Hits 3785 3786 +1
Misses 146 146
☔ View full report in Codecov by Sentry. |
| @@ -90,6 +97,9 @@ def test___init__(self, update_transformer_mock, mock_rdt): | |||
| assert data_processor._hyper_transformer == mock_rdt.HyperTransformer.return_value | |||
| update_transformer_mock.assert_called_with(True, False) | |||
|
|
|||
| mock_default_transformers.assert_called_once() | |||
There was a problem hiding this comment.
you should mock the return value and make sure the id default is properly added and that the text one is deleted
There was a problem hiding this comment.
Yes done in d84def8
| assert 'text' not in data_processor._transformers_by_sdtype | ||
| assert 'id' in data_processor._transformers_by_sdtype | ||
| assert data_processor._transformers_by_sdtype == expected_default_transformers |
There was a problem hiding this comment.
the last assert is probably all you need
There was a problem hiding this comment.
Yes, done in 9a01726
| if version: | ||
| version = re.sub(r'>=?', '==', version) | ||
| version = re.sub(r"""['",]""", '', version) | ||
| requirement += version | ||
| versions.append(requirement) | ||
|
|
||
| elif (line.startswith('install_requires = [') or | ||
| line.startswith('pomegranate_requires = [')): | ||
| elif (line.startswith('install_requires = [')): |
There was a problem hiding this comment.
can you leave a comment in the description of the PR explaining why we made this change and the one on line 74? If we're going to merge this in this PR it would be good to understand why
Resolve #1484
Made a change in tasks.py to include
.devversion of libraries and made the minimum version GitHub action pass.