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

[ENH]row_to_names for polars #1363

Merged
merged 58 commits into from
Jun 11, 2024
Merged

[ENH]row_to_names for polars #1363

merged 58 commits into from
Jun 11, 2024

Conversation

samukweku
Copy link
Collaborator

@samukweku samukweku commented May 20, 2024

PR Description

Please describe the changes proposed in the pull request:

  • Add row_to_names method for polars

**This PR relates to #1352 **

PR Checklist

Please ensure that you have done the following:

  1. PR in from a fork off your branch. Do not PR from <your_username>:dev, but rather from <your_username>:<feature-branch_name>.
  1. If you're not on the contributors list, add yourself to AUTHORS.md.
  1. Add a line to CHANGELOG.md under the latest version header (i.e. the one that is "on deck") describing the contribution.
    • Do use some discretion here; if there are multiple PRs that are related, keep them in a single line.

Automatic checks

There will be automatic checks run on the PR. These include:

  • Building a preview of the docs on Netlify
  • Automatically linting the code
  • Making sure the code is documented
  • Making sure that all tests are passed
  • Making sure that code coverage doesn't go down.

Relevant Reviewers

Please tag maintainers to review.

@ericmjl
Copy link
Member

ericmjl commented May 20, 2024

@ericmjl
Copy link
Member

ericmjl commented Jun 3, 2024

@samukweku it looks like this one also needs to have merge conflicts resolved before we can continue. Would you be kind enough to take a look at it please?

@ericmjl
Copy link
Member

ericmjl commented Jun 9, 2024

@samukweku it looks like merging other polars dataframe functions results in large merge conflicts, which may make further development on the polars implementation of functions a bit more tricky. We may need to think carefully about how the code is organized here.

Do you think there is a way to leverage the polars API to create a polars-flavor package, not unlike the pandas-flavor package that we currently maintain? I think that would be the easiest way for us to handle the matter.

I tried thinking of a few other strategies, but none of them seem satisfactory. This included dynamically monkey-patching the docstrings of the PolarsFrame class methods from the underlying implementation function. But that has problems, just like other ideas I thought of, and didn't seem satisfactory.

@samukweku
Copy link
Collaborator Author

@ericmjl i thought of going with the pandas_flavor format but decided against it (polars has three namespaces which we support - lazyframe, expr and eager frames - it would be an issue in the docs rendering for these namespaces) - so i chose another route, creating 3 files for each of the namespaces. Still not sure if this is good enough - hopefully there is a better way. or maybe there is a better way to modify pandas_flavor to better support the namespaces?

@ericmjl
Copy link
Member

ericmjl commented Jun 11, 2024

@samukweku that's a good point, I didn't think of that.

@ericmjl ericmjl merged commit 5c281b2 into dev Jun 11, 2024
4 checks passed
@ericmjl ericmjl deleted the samukweku/polars_row_names branch June 11, 2024 00:46
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.

None yet

2 participants