DEPR: Change default value for CategoricalDtype.ordered from None to False #29955
Conversation
jschendel
added
Categorical
There was a problem hiding this comment.
I don't fully remember the previous discussion (should look back at it), but do we want to allow ordered=None in the main constructor? We need it internally to denote a categorical dtype that should not override the ordered property of another one, but do we need to expose this in the public constructor? (could also have a private way to created this)
If we keep it in the public constructor, we should probably document it?
doc/source/whatsnew/v1.0.0.rst
Outdated
| @@ -519,7 +519,7 @@ or ``matplotlib.Axes.plot``. See :ref:`plotting.formatters` for more. | |||
| - Changed :meth:`Timedelta.resolution` to match the behavior of the standard library ``datetime.timedelta.resolution``, for the old behavior, use :meth:`Timedelta.resolution_string` (:issue:`26839`) | |||
| - Removed previously deprecated :attr:`Timestamp.weekday_name`, :attr:`DatetimeIndex.weekday_name`, and :attr:`Series.dt.weekday_name` (:issue:`18164`) | |||
| - Removed previously deprecated ``errors`` argument in :meth:`Timestamp.tz_localize`, :meth:`DatetimeIndex.tz_localize`, and :meth:`Series.tz_localize` (:issue:`22644`) | |||
| - | |||
| - Changed the default value for ``ordered`` in :class:`~pandas.api.types.CategoricalDtype` from ``None`` to ``False`` (:issue:`26336`) | |||
There was a problem hiding this comment.
| - Changed the default value for ``ordered`` in :class:`~pandas.api.types.CategoricalDtype` from ``None`` to ``False`` (:issue:`26336`) | |
| - Changed the default value for ``ordered`` in :class:`~pandas.CategoricalDtype` from ``None`` to ``False`` (:issue:`26336`) |
(that's the public path as in the api docs: https://dev.pandas.io/docs/reference/api/pandas.CategoricalDtype.html)
There was a problem hiding this comment.
Thanks, updated to :class:`CategoricalDtype`
pandas/core/dtypes/dtypes.py
Outdated
| "must be explicitly passed in order to be retained" | ||
| ) | ||
| warnings.warn(msg, FutureWarning, stacklevel=3) | ||
| new_ordered = self.ordered |
There was a problem hiding this comment.
Can you add a comment here why ordered can be None?
There was a problem hiding this comment.
There's a comment a little further up that actually addresses this (above the new_categories definition) but as written it's not immediately noticeable. I've shortened the logic for both of these to be ternary statements, which hopefully makes the comment is more observable, and cleaned up the wording of the comment.
I had forgotten too but apparently my past self anticipated this and wrote a nice summary here: #26403 (comment). Also edited my first comment so this link in included at the top of the PR. Looks like the decision was to maintain the
I can add some documentation for this in a bit (after sleep + work) if we want to advertise this behavior. There were previously some concerns that this might not be behavior we want to encourage (xref #26403 (comment)) since this is in part an implementation detail to handle |
|
@jschendel needs rebase |
|
merged master and added a note regarding the usage of |
|
@jschendel Thanks for the update and explanation ! |
black pandasgit diff upstream/master -u -- "*.py" | flake8 --diffA quick summary of the deprecation details can be found here: #26403 (comment)