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: tuple supported for getitem for Pandas dataframes #1026

Conversation

mikeweltevrede
Copy link
Contributor

@mikeweltevrede mikeweltevrede commented Sep 20, 2024

What type of PR is this? (check all applicable)

  • πŸ’Ύ Refactor
  • ✨ Feature
  • πŸ› Bug Fix
  • πŸ”§ Optimization
  • πŸ“ Documentation
  • βœ… Test
  • 🐳 Other

Related issues

Checklist

  • Code follows style guide (ruff)
  • Tests added
  • Documented the changes -> Documented through descriptive tests and this PR, please let us know if documentation in another location is needed.

If you have comments or can explain your changes, please do so below.

To close #990 we now make sure that Pandas-like Dataframes can be indexed with tuples to support Polars-like behaviour.

@github-actions github-actions bot added the fix label Sep 20, 2024
@mikeweltevrede mikeweltevrede changed the title Fix/tuple supported for getitem/dev Fix: tuple supported for getitem for Pandas dataframes Sep 20, 2024
@mikeweltevrede mikeweltevrede changed the title Fix: tuple supported for getitem for Pandas dataframes fix: tuple supported for getitem for Pandas dataframes Sep 20, 2024
@windiana42
Copy link
Contributor

This PR is ready for review.

.gitignore Outdated
@@ -34,6 +34,3 @@ tpch/data/*

# MacOS
.DS_Store

# IntelliJ IDEA
.idea/
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Riik why was this removed? I would consider this the same as .vscode but then for IntelliJ and Pycharm. It should not be pushed.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree it would be easier to have .idea in gitginore as well, but figured it was easier to keep that separate from this PR since it was just about the bug ticket

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove the README clean up then too :)

@mikeweltevrede mikeweltevrede marked this pull request as ready for review September 20, 2024 15:57
@mikeweltevrede
Copy link
Contributor Author

@MarcoGorelli @Riik @windiana42 PS: Here is the video that showed me that typing is really just aliasing collections for a lot of type hints: https://www.youtube.com/watch?v=cv1F_c66utw

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

@MarcoGorelli MarcoGorelli merged commit 6b6fefd into narwhals-dev:main Sep 21, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: Can not subset using tuple()
4 participants