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

chore(python): Deprecate positional join args #6826

Merged
merged 3 commits into from
Feb 13, 2023

Conversation

stinodego
Copy link
Member

@stinodego stinodego commented Feb 12, 2023

This is a PoC of sorts to deprecate positional arguments.

As an example, the function signature for join is as follows:

    def join(
        other,
        left_on = None,
        right_on = None,
        on = None,
        how = "inner",
        suffix = "_right",
    ) -> DataFrame:

I think the function signature below would make more sense, putting the most-used args first, and make others keyword-only:

    def join(
        other,
        on = None,
        how = "inner",
        *,
        left_on = None,
        right_on = None,
        suffix = "_right",
    ) -> DataFrame:

This is obviously a breaking change, but worse, this may silently break for people who have specified left_on and right_on positionally.

I came up with a way to detect such cases and throw a DeprecationWarning

I stole the decorator from Pandas (as suggested by @MarcoGorelli) and adapted it to our needs.

If you agree with this method of deprecating non-keyword arguments, I will go ahead and apply this to a bunch of other functions.

Changes:

  • Add a decorator utility for deprecating non-keyword arguments.
  • Use the decorator to deprecate positional arguments for the join method.

@github-actions github-actions bot added chore Maintenance work that does not impact the user python Related to Python Polars labels Feb 12, 2023
@stinodego stinodego force-pushed the deprecate-join branch 2 times, most recently from 34e052a to e315b26 Compare February 13, 2023 03:57
@stinodego stinodego marked this pull request as ready for review February 13, 2023 04:01
@ritchie46
Copy link
Member

This is great. These things really enhance readability and maintainability. 👍

@ritchie46 ritchie46 merged commit 4649a0e into pola-rs:master Feb 13, 2023
@stinodego stinodego deleted the deprecate-join branch February 22, 2023 18:34
@timreuscher
Copy link

timreuscher commented Apr 13, 2023

This one just really bit me, and the API reference doesn't have a decent example. In 0.16.4 I had something like this:

 df.join(other_df,vars1,vars2)

That worked great for me. I understand the reason for the change completely, but something like

 df.join(other_df,on=None,how='inner',left_on=vars1,right_on=vars2)

gives me an error for multiple values for argument 'how'. Like I said, is there an example for how this decorator works? I'm not really understanding how to work with the change.

@MarcoGorelli
Copy link
Collaborator

I can't reproduce this, could you show a reproducible example please?

In [31]: df.join(df2, on=None, how='inner', left_on=['a'], right_on=['b'])
Out[31]:
shape: (3, 3)
┌─────┬─────┬─────────┐
│ aba_right │
│ ---------     │
│ i64i64i64     │
╞═════╪═════╪═════════╡
│ 111       │
│ 121       │
│ 231       │
└─────┴─────┴─────────┘

@timreuscher
Copy link

I can't reproduce this, could you show a reproducible example please?

In [31]: df.join(df2, on=None, how='inner', left_on=['a'], right_on=['b'])
Out[31]:
shape: (3, 3)
┌─────┬─────┬─────────┐
│ aba_right │
│ ---------     │
│ i64i64i64     │
╞═════╪═════╪═════════╡
│ 111       │
│ 121       │
│ 231       │
└─────┴─────┴─────────┘

OK, sorry about that. That code does work. I had multiple joins in my code and the error was in the next join. That "*" really threw me off. My apologies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Maintenance work that does not impact the user python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants