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

Remove the experimental $expr subnamespace for Series #827

Closed
wants to merge 2 commits into from

Conversation

etiennebacher
Copy link
Collaborator

Follow-up of #819

Comment on lines +25 to +27
- The experimental subnamespace `$expr` on `Series` is removed as it was
barely documented and doesn't have additional value compared to applying
methods directly on `Series` (#827).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sorry I have not answered your question adequately.
In fact, under $expr, all the methods of the Expr class are stored, so there are functions that cannot be used without it.

In Python, the methods of the Series class are almost the same as what we do with $expr, since the methods of Expr are dispatched as follows.
https://github.com/pola-rs/polars/blob/24c0f474108e6dcf095068823ba0925b31c4e3b4/py-polars/polars/series/utils.py#L28-L36

So we should aim to be able to do the same thing without $expr, and until that is achieved $expr may be worthwhile.
But as I commented elsewhere, I think very few users are using this at the moment.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah ok I see, the example I mentioned in another PR only worked because $add() was already implemented for Series. I'll think more about how we can automatically dispatch the other methods without using $expr. Thanks for the explanation

Copy link
Collaborator

Choose a reason for hiding this comment

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

So we should aim to be able to do the same thing without $expr

Done in #831

@eitsupi eitsupi deleted the drop-expr-series branch February 22, 2024 02:50
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.

2 participants