-
Notifications
You must be signed in to change notification settings - Fork 47
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
feat: add replace method to DataFrame #261
Conversation
5f80862
to
4bc2753
Compare
to_replace (str, regex, list, int, float or None): | ||
How to find the values that will be replaced. | ||
|
||
* numeric, str or regex: |
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 think the bulletpoint will be rendered correctly, we should get rid of it. Could you help do a staging following the instructions: https://screenshot.googleplex.com/5MQFPth8968Qjtw.
Just like: https://screenshot.googleplex.com/BPQxVeg7D7NjjTZ
Sorry, something went wrong.
All reactions
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.
removed bullet points
Sorry, something went wrong.
All reactions
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.
Not sure how the CloudRAD tool will render this. Will LGTM for this one, and we can check back if we need to re-format it.
Sorry, something went wrong.
All reactions
regex=False, | ||
): | ||
""" | ||
Replace values given in `to_replace` with `value`. |
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.
Could you also help add code_samples in the docs as well? Since the person who implements the method knows more about the use cases, and we don't need to do it later in a seperate PR.
Sorry, something went wrong.
All reactions
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 a few code samples
Sorry, something went wrong.
All reactions
|
||
|
||
def is_patype(scalar: typing.Any, pa_type: pa.DataType) -> bool: | ||
if pa_type == pa.time64("us"): |
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.
Missing docstring here?
Sorry, something went wrong.
All reactions
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 docstring
Sorry, something went wrong.
All reactions
ashleyxuu
Successfully merging this pull request may close these issues.
None yet
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕