Skip to content
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

CLN/DEPR: removed deprecated as_indexer arg from str.match() #22356

Closed
wants to merge 9 commits into from

Conversation

Projects
None yet
4 participants
@HyunTruth
Copy link
Contributor

commented Aug 15, 2018

  • closes #22316
  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

removed as_indexer(deprecated of 0.20.0) arg completely from str.match.

@@ -630,7 +630,7 @@ Numeric
Strings
^^^^^^^

-
- Removed as_indexer(deprecated of 0.21.0) keyword completely from str.match() (:issue:`22356`,:issue:`6581`)

This comment has been minimized.

Copy link
@gfyoung

gfyoung Aug 15, 2018

Member
  • You mean 0.20.0 ? You can also just remove that parenthetical comment.
  • This should be in the Removal of prior version deprecations/changes section.

This comment has been minimized.

Copy link
@HyunTruth

HyunTruth Aug 15, 2018

Author Contributor

thanks, will fix it straight away

with tm.assert_produces_warning(FutureWarning):
with tm.assert_raises_regex(TypeError,
"match() got an unexpected "
"keyword argument 'as_indexer'"):

This comment has been minimized.

Copy link
@gfyoung

gfyoung Aug 15, 2018

Member

I like the thought. However, we generally just drop all of the relevant tests when a keyword is getting dropped after a deprecation.

@gfyoung gfyoung changed the title removed as_indexer(deprecated of 0.21.0) arg completely from str.match() removed as_indexer(deprecated of 0.20.0) arg completely from str.match() Aug 15, 2018

@codecov

This comment has been minimized.

Copy link

commented Aug 15, 2018

Codecov Report

Merging #22356 into master will decrease coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #22356      +/-   ##
==========================================
- Coverage   92.05%   92.05%   -0.01%     
==========================================
  Files         169      169              
  Lines       50709    50705       -4     
==========================================
- Hits        46679    46675       -4     
  Misses       4030     4030
Flag Coverage Δ
#multiple 90.46% <100%> (-0.01%) ⬇️
#single 42.26% <66.66%> (ø) ⬆️
Impacted Files Coverage Δ
pandas/core/strings.py 98.62% <100%> (-0.01%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf70d11...b109d19. Read the comment docs.

@jreback

This comment has been minimized.

Copy link
Contributor

commented Aug 16, 2018

see my comment: #22316 (comment)

I am not sure we can remove this for 0.24.0

@@ -722,8 +722,6 @@ def str_match(arr, pat, case=True, flags=0, na=np.nan, as_indexer=None):
flags : int, default 0 (no flags)
re module flags, e.g. re.IGNORECASE
na : default NaN, fill value for missing values.
as_indexer
.. deprecated:: 0.21.0

This comment has been minimized.

Copy link
@jreback

jreback Aug 16, 2018

Contributor

so is this just wrong?

This comment has been minimized.

Copy link
@jreback

jreback Aug 22, 2018

Contributor

@HyunTruth can you see when this was added? if it was in 0.21.0 then we can't remove this yet, if it was a mistake / typo then we could.

This comment has been minimized.

Copy link
@HyunTruth

HyunTruth Aug 22, 2018

Author Contributor

Sure, I'll check on it.

This comment has been minimized.

Copy link
@HyunTruth

HyunTruth Aug 22, 2018

Author Contributor

@jreback It seems to be a typo, as in whatsnew v0.20.0, it is stated that

- The default behaviour of ``Series.str.match`` has changed from extracting
  groups to matching the pattern. The extracting behaviour was deprecated
  since pandas version 0.13.0 and can be done with the ``Series.str.extract``
  method (:issue:`5224`). As a consequence, the ``as_indexer`` keyword is
  ignored (no longer needed to specify the new behaviour) and is deprecated.

This comment has been minimized.

Copy link
@jreback

jreback Sep 4, 2018

Contributor

ok then

@HyunTruth

This comment has been minimized.

Copy link
Contributor Author

commented Aug 17, 2018

@jreback What do you suggest as the next course of action on this issue?

@jreback

This comment has been minimized.

Copy link
Contributor

commented Sep 4, 2018

can you rebase

@jorisvandenbossche jorisvandenbossche changed the title removed as_indexer(deprecated of 0.20.0) arg completely from str.match() CLN/DEPR: removed deprecated as_indexer arg from str.match() Sep 5, 2018

@jorisvandenbossche
Copy link
Member

left a comment

Apart from fixing the conflicts (updating with latest master), looks good to me!

@HyunTruth

This comment has been minimized.

Copy link
Contributor Author

commented Sep 7, 2018

Pull request moved to #22626

@HyunTruth HyunTruth closed this Sep 7, 2018

@jorisvandenbossche jorisvandenbossche added this to the No action milestone Sep 7, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.