-
-
Notifications
You must be signed in to change notification settings - Fork 17.4k
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
API: Deprecate regex=True default in Series.str.replace #36695
Merged
Merged
Changes from 14 commits
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
52f6d78
API: Deprecate regex=True default in Series.str.replace
3b9bcb2
Format
c51e837
Stacklevel
3761625
Test
7583b7f
Test name
8e0d1a5
Link discussion
79e4400
Set regex
ecc5786
Update
cf659b0
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
628cfa3
Update
f216022
Fix
e6c79a3
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
e6b3e0f
Check for str
e3ce080
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
752d322
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
ee67284
Doc updates
c832a21
Nit
09b07e6
Silence some warnings
c8541da
Edit
bd31857
Edit
377a2ba
Question mark
d6b4fbe
Update doc
84dfe71
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
5cfbc04
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
1b53051
Add back
1931c31
Oops
20bfe16
Note
6ff5955
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
cea44a7
Fix
4376bca
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
482d5c3
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
cd18347
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
f872011
Update warning
c0a473e
rst lint
727986e
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
f49f778
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
d3d155a
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
89013db
Nit + emphasis
e78017c
regex=False
0150b2b
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
e396ce5
Update
e799b12
fix
9f7545f
rst fix
6be90a4
Merge remote-tracking branch 'upstream/master' into replace-regex-def…
8a4a833
Make a warning
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -449,3 +449,9 @@ def test_replace_with_compiled_regex(self): | |
result = s.replace({regex: "z"}, regex=True) | ||
expected = pd.Series(["z", "b", "c"]) | ||
tm.assert_series_equal(result, expected) | ||
|
||
def test_str_replace_regex_default_raises_warning(self): | ||
# https://github.com/pandas-dev/pandas/pull/24809 | ||
s = pd.Series(["a", "b", "c"]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can you check the messages on this warning |
||
with tm.assert_produces_warning(FutureWarning, check_stacklevel=False): | ||
s.str.replace("^.$", "") |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
you can make this a note / warning
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.
can you do this
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.
also add a versionchanged tag
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 versionchanged tag. Do we also need one in the docstring?
I'm not sure what's meant by note / warning since I've added a whatsnew note and warning. Do you mean put the text itself in the whatsnew?
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.
no i mean a
..note::
a sphinx-note (which puts a highlite box around this)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.
we could also do a
..warning::
which is a red-box and more prominent, but either way want to call out thisThere 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.
Made this a .. warning