-
Notifications
You must be signed in to change notification settings - Fork 17
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
feature/StringConcatenator #52
feature/StringConcatenator #52
Conversation
test module and example notebook added.
tubular/strings.py
Outdated
|
||
def transform(self, X): | ||
""" | ||
Concatenates values into one string. |
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.
Again these descriptions feel a bit vague
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.
"metadata": {}, | ||
"source": [ | ||
"# StringConcatenator\n", | ||
"This notebook shows the functionality of the `StringConcatenator` class. This transformer joins values from different columns into a new string in a new column." |
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.
same again here if we're changing values here to something more specific
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.
tubular/strings.py
Outdated
|
||
class StringConcatenator(BaseTransformer): | ||
""" | ||
Transformer to concatenate values from a few columns into 1 string. |
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.
Sorry the first comment (repeated below) should have been about the "concatenate values from a few columns" in this docstring - all the others reference this with "again"
This feels a bit vague - not immediately clear that values can be any datatype, maybe:
Transform to combine data from specified columns, of mixed datatypes, into a new column containing one string?
(will this work on e.g. a column of dictionaries?)
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.
"cell_type": "markdown", | ||
"metadata": {}, | ||
"source": [ | ||
"# StringConcatenator\n", | ||
"This notebook shows the functionality of the `StringConcatenator` class. This transformer joins values from different columns into a new string in a new column." | ||
"This notebook shows the functionality of the `StringConcatenator` class. Thjis Transformer combines data from specified columns, of mixed datatypes, into a new column containing one string." |
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.
typo
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.
Added transfoormer StringConcatenator to strings.py. It concatenates input coolumns into a single string output column.
Added test module and example notebook.