-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
DEPR: Deprecate ordered=None for CategoricalDtype #26403
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
Merged
Merged
Changes from 1 commit
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
adc0bca
DEPR: Deprecate ordered=None for CategoricalDtype
jschendel 4ad16c9
Merge remote-tracking branch 'upstream/master' into depr-cdt-ordered-…
jschendel 2da967d
cover additional case and review edits
jschendel b03eadd
fix whatsnew
jschendel 77f171d
update warning message
jschendel cf12a1f
consolidate special casing
jschendel 060eb08
Merge remote-tracking branch 'upstream/master' into depr-cdt-ordered-…
jschendel c9c2333
Merge remote-tracking branch 'upstream/master' into depr-cdt-ordered-…
jschendel bc16e98
.ordered warning and keep None as non-default
jschendel 99b6a30
fix typing and docs
jschendel d999bd8
sentinel --> ordered_sentinel
jschendel fdb5770
add more detail to warning
jschendel c45f159
Merge branch 'master' into depr-cdt-ordered-none
jreback 219f03c
Merge branch 'master' into PR_TOOL_MERGE_PR_26403
jreback File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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 hidden or 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 hidden or 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 hidden or 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 |
---|---|---|
|
@@ -817,7 +817,13 @@ def test_update_dtype(self, ordered_fixture, new_categories, new_ordered): | |
if expected_ordered is None: | ||
expected_ordered = dtype.ordered | ||
|
||
result = dtype.update_dtype(new_dtype) | ||
# GH 26336 | ||
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 give some explanation on what you are testing here (the cases) |
||
if new_ordered is None and ordered_fixture is True: | ||
with tm.assert_produces_warning(FutureWarning): | ||
result = dtype.update_dtype(new_dtype) | ||
else: | ||
result = dtype.update_dtype(new_dtype) | ||
|
||
tm.assert_index_equal(result.categories, expected_categories) | ||
assert result.ordered is expected_ordered | ||
|
||
|
This file contains hidden or 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 hidden or 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
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.
Maybe we should let the stacklevel fit for the astype case (although the series and index case might need a different stacklevel), as that seems the more common case compared to directly using this method?
Or, otherwise, I would certainly try to make the context clearer in this warning message: indicate that a CategoricalDtype was constructed without specifying the ordered, etc (otherwise the message might be very confusing, as it is not raised when creating it)
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've updated the
stacklevel
so that it works for theCategorical.astype
andCategoricalIndex.astype
cases; all other cases look like they require a uniquestacklevel
. I've also updated the message itself to be more clear.