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

fix: update pivot expr on to first param #252

Merged
merged 2 commits into from
Aug 16, 2024

Conversation

b-twice
Copy link
Contributor

@b-twice b-twice commented Aug 10, 2024

The pivot function now has on as the first param instead of index, as noted in the python 1.0 upgrade documentation:

columns has been renamed to on, and is now the first positional argument.

image

Added some test coverage from examples in docs to capture this. Existing test coverage was using the same column as index and on, resulting in this not being caught.

@b-twice b-twice changed the title update pivot on to first pos argument fix: update pivot expr on to first param Aug 10, 2024
@Bidek56
Copy link
Collaborator

Bidek56 commented Aug 11, 2024

  1. Don't you have to change the Rust fn pivot_expr( to match it?
  2. Did you run 'yarn run lints:ts` on your TS code?
    Thx

@b-twice
Copy link
Contributor Author

b-twice commented Aug 11, 2024

  1. Don't you have to change the Rust fn pivot_expr( to match it?

    1. Did you run 'yarn run lints:ts` on your TS code?
      Thx
  1. 🤦‍♂️That would be helpful... done
  2. Yes passes lint + tests:
Screenshot 2024-08-11 at 7 29 39 PM

@universalmind303 universalmind303 merged commit eae13e8 into pola-rs:main Aug 16, 2024
9 checks passed
@owlas
Copy link

owlas commented Sep 30, 2024

Thanks for fixing this, our team was so confused that index + on where reversed. Is this in the latest release?

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

Successfully merging this pull request may close these issues.

4 participants