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
Add semantic tag selection #106
Conversation
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.
looks good overall. just minor things.
67dfdd5
to
666a187
Compare
666a187
to
a7dd9a2
Compare
Codecov Report
@@ Coverage Diff @@
## main #106 +/- ##
==========================================
+ Coverage 99.58% 99.62% +0.04%
==========================================
Files 16 18 +2
Lines 959 1081 +122
==========================================
+ Hits 955 1077 +122
Misses 4 4
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.
Just 1 thing
woodwork/data_column.py
Outdated
@@ -69,12 +70,12 @@ def _parse_logical_type(self, logical_type): | |||
|
|||
def set_semantic_tags(self, semantic_tags): | |||
"""Replace semantic tags with passed values""" | |||
self._semantic_tags = _parse_semantic_tags(semantic_tags) | |||
self._semantic_tags = _parse_semantic_tags(semantic_tags, 'semantic_tags') |
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.
maybe put _parse_semantic_tags(semantic_tags, error_language='semantic_tags')
. So we don't get confused why its listed twice.
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.
Yeah, that was something I'd been thinking about too. Also changed one of the tests to test an error message with test_text
as the optional parameter
Closes #82