-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
PR: Catch search regexp error while doing replace/find #5618
Conversation
Maybe you could generalize the function
|
Hello @timhoffm! Thanks for updating the PR.
|
@timhoffm, don't worry about PEP8 issues in our test files. You can safely ignore |
@ccordoba12 If you don't want
to |
Good to know! Will do that in another PR (I looked how to do it some time ago but didn't find a straight answer). |
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.
Thanks @timhoffm, nice work I think everything its ok. I also tested locally and everything regarding your changes works as expected 👍
I agree, let's merge this one first. I think there's an error with Pandas that's making our tests to fail, so I'll take a look at that first before merging more work. |
@rlaverde, @ccordoba12, @dalthviz I would do one step at a time. Both this PR and #5614 are reasonable solutions in themselves and can be merged as they are. In a second step, we can then see if a generalization of |
yes, that's true, but in both the try..except is repeated
|
Let's leave a further refactoring for another PR. |
fixes #5386
Perfroming
replace_find()
andreplace_find_selection()
with an invalid regexp used to raise an exception. After this patch, they will do nothing and silently return.