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

DEPR: make Categorical.ravel() return Categorical #27199

Merged
merged 7 commits into from Jul 3, 2019

Conversation

@jbrockmendel
Copy link
Member

@jbrockmendel jbrockmendel commented Jul 3, 2019

Discussed in #27142.

  • closes #xxxx
  • tests added / passed
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry
@jreback
Copy link
Contributor

@jreback jreback commented Jul 3, 2019

conflict and failing

pandas/tests/extension/test_categorical.py(100,): error :  Found unwanted pattern:         with pytest.warns(FutureWarning, match="will return a Categorical")

otherwise lgtm.

@@ -2,6 +2,9 @@


class BaseExtensionTests:
# Whether the EA being tested supports __setitem__
_supports_setitem = True
Copy link
Contributor

@jreback jreback Jul 3, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this is ok, but in reality you should have a class which does this (future PR)

@jreback
Copy link
Contributor

@jreback jreback commented Jul 3, 2019

I am a little concerned by the change in testing pattern this introduces. Could we remove this for now.

@jbrockmendel
Copy link
Member Author

@jbrockmendel jbrockmendel commented Jul 3, 2019

testing pattern reverted

@jreback
Copy link
Contributor

@jreback jreback commented Jul 3, 2019

lgtm. merge on green.

@jreback jreback merged commit 1659fff into pandas-dev:master Jul 3, 2019
14 checks passed
@jreback
Copy link
Contributor

@jreback jreback commented Jul 3, 2019

thanks @jbrockmendel

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants