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

Deprecate passing args as positional in sort_values #41505

Merged

Conversation

MarcoGorelli
Copy link
Member

@MarcoGorelli MarcoGorelli added the Deprecate Functionality to remove in pandas label May 16, 2021
@MarcoGorelli MarcoGorelli force-pushed the deprecate-nonkeyword-args-sort_values branch from d8da296 to 2a0270e Compare May 16, 2021 12:36
@@ -647,6 +647,7 @@ Deprecations
- Deprecated setting :attr:`Categorical._codes`, create a new :class:`Categorical` with the desired codes instead (:issue:`40606`)
- Deprecated behavior of :meth:`DatetimeIndex.union` with mixed timezones; in a future version both will be cast to UTC instead of object dtype (:issue:`39328`)
- Deprecated using ``usecols`` with out of bounds indices for ``read_csv`` with ``engine="c"`` (:issue:`25623`)
- Deprecated passing arguments as positional (except for ``by``) in :meth:`DataFrame.sort_values` and :meth:`Series.sort_values` (:issue:`41485`)
Copy link
Member

Choose a reason for hiding this comment

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

this change won't solve the error: Signature of "sort_values" incompatible with supertype "NDFrame" [override]. maybe we could also give by a default value (the dataframe columns). The 'by' keyword could still be passed positionally if it has a default value.

Copy link
Member

Choose a reason for hiding this comment

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

we could also perhaps deprecate the axis keyword on Series at the same time to clean this up further.

Copy link
Member Author

Choose a reason for hiding this comment

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

we could also perhaps deprecate the axis keyword on Series at the same time to clean this up further.

I don't understand, wouldn't this be incompatible with the supertype?

maybe we could also give by a default value (the dataframe columns). The 'by' keyword could still be passed positionally if it has a default value.

Sure, could that - that could be done directly in 1.3 without a prior warning, right? As current behaviour would be preserved

Copy link
Member

Choose a reason for hiding this comment

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

maybe we could also give by a default value (the dataframe columns). The 'by' keyword could still be passed positionally if it has a default value.

That might be a good idea, but I would personally leave that for a separate PR (as it seems an independent new feature)

@@ -647,6 +647,7 @@ Deprecations
- Deprecated setting :attr:`Categorical._codes`, create a new :class:`Categorical` with the desired codes instead (:issue:`40606`)
- Deprecated behavior of :meth:`DatetimeIndex.union` with mixed timezones; in a future version both will be cast to UTC instead of object dtype (:issue:`39328`)
- Deprecated using ``usecols`` with out of bounds indices for ``read_csv`` with ``engine="c"`` (:issue:`25623`)
- Deprecated passing arguments as positional (except for ``by``) in :meth:`DataFrame.sort_values` and :meth:`Series.sort_values` (:issue:`41485`)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Deprecated passing arguments as positional (except for ``by``) in :meth:`DataFrame.sort_values` and :meth:`Series.sort_values` (:issue:`41485`)
- Deprecated passing arguments as positional in :meth:`DataFrame.sort_values` (except for ``by``) and :meth:`Series.sort_values` (:issue:`41485`)

r"for the arguments 'self' and 'by' will be keyword-only"
)
with tm.assert_produces_warning(FutureWarning, match=msg):
df.sort_values("a", 0)
Copy link
Member

Choose a reason for hiding this comment

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

Same comment as in other PRs regarding asserting the result

@jorisvandenbossche jorisvandenbossche changed the title deprecate passing args as positional in sort_values Deprecate passing args as positional in sort_values May 17, 2021
@MarcoGorelli MarcoGorelli marked this pull request as draft May 18, 2021 08:14
@MarcoGorelli MarcoGorelli force-pushed the deprecate-nonkeyword-args-sort_values branch from af0bf72 to fee3211 Compare May 18, 2021 21:03
@MarcoGorelli MarcoGorelli marked this pull request as ready for review May 18, 2021 21:03
@simonjayhawkins simonjayhawkins added this to the 1.3 milestone May 24, 2021
@jreback jreback merged commit 9f65984 into pandas-dev:master May 26, 2021
@MarcoGorelli MarcoGorelli deleted the deprecate-nonkeyword-args-sort_values branch May 26, 2021 20:51
TLouf pushed a commit to TLouf/pandas that referenced this pull request Jun 1, 2021
JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Deprecate Functionality to remove in pandas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants