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

FEAT-#7331: Initial Polars API #7332

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

devin-petersohn
Copy link
Collaborator

@devin-petersohn devin-petersohn commented Jun 26, 2024

This commit adds a polars namespace to Modin, and the DataFrame and Series objects and their respective APIs.

This PR adds the following to Modin:

  • modin.polars.DataFrame
  • modin.polars.Series
  • modin.polars.GroupBy

This PR does NOT include matching error handling and is still missing several polars features:

  • LazyFrame
  • Expressions
  • String, Temporal, Struct, and other Series accessors
  • Several parameters
  • Operators that we don't have query compiler methods for
    • e.g. sin, cos, tan, etc.

These features will be added in a future PR.

What do these changes do?

  • first commit message and PR title follow format outlined here

    NOTE: If you edit the PR title to match this format, you need to add another commit (even if it's empty) or amend your last commit for the CI job that checks the PR title to pick up the new PR title.

  • passes flake8 modin/ asv_bench/benchmarks scripts/doc_checker.py
  • passes black --check modin/ asv_bench/benchmarks scripts/doc_checker.py
  • signed commit with git commit -s
  • Resolves Polars API #7331
  • tests added and passing
  • module layout described at docs/development/architecture.rst is up-to-date

This commit adds a polars namespace to Modin, and the DataFrame and
Series objects and their respective APIs. This doesn't include error
handling and is still missing several polars features:

* LazyFrame
* Expressions
* String, Temporal, Struct, and other Series accessors
* Several parameters
* Operators that we don't have query compiler methods for
   * e.g. sin, cos, tan, etc.

Those will be handled in a future PR.

Signed-off-by: Devin Petersohn <[email protected]>
Signed-off-by: Devin Petersohn <[email protected]>
modin/polars/dataframe.py Dismissed Show dismissed Hide dismissed
modin/polars/series.py Fixed Show fixed Hide fixed
modin/polars/dataframe.py Fixed Show fixed Hide fixed
modin/polars/groupby.py Fixed Show fixed Hide fixed
modin/polars/base.py Fixed Show fixed Hide fixed
modin/polars/base.py Fixed Show fixed Hide fixed
modin/polars/dataframe.py Fixed Show fixed Hide fixed
modin/polars/series.py Fixed Show fixed Hide fixed
Signed-off-by: Devin Petersohn <[email protected]>
Signed-off-by: Devin Petersohn <[email protected]>
modin/polars/series.py Dismissed Show dismissed Hide dismissed
Signed-off-by: Devin Petersohn <[email protected]>
Signed-off-by: Devin Petersohn <[email protected]>
Signed-off-by: Devin Petersohn <[email protected]>
Signed-off-by: Devin Petersohn <[email protected]>
Signed-off-by: Devin Petersohn <[email protected]>
Signed-off-by: Devin Petersohn <[email protected]>
Signed-off-by: Devin Petersohn <[email protected]>
Copy link
Contributor

@sfc-gh-mvashishtha sfc-gh-mvashishtha left a comment

Choose a reason for hiding this comment

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

I reviewed the files alphabetically through item in dataframe.py . I'll make another round of comments later today.

def __eq__(self, other) -> "BasePolarsDataset":
return self.__constructor__(
_query_compiler=self._query_compiler.eq(
other._query_compiler if hasattr(other, "_query_compiler") else other
Copy link
Contributor

Choose a reason for hiding this comment

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

optional comment: Why look for query_compiler instead of checking whether other is a BasePolarsDataset?

Returns:
Arrow representation of the DataFrame.
"""
return polars.from_pandas(self._query_compiler.to_pandas()).to_arrow()
Copy link
Contributor

Choose a reason for hiding this comment

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

optional nit: should we add a _to_polars() to alias polars.from_pandas(self._query_compiler.to_pandas())?

Copy link
Contributor

Choose a reason for hiding this comment

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

And should we replace all the polars.from_pandas(self._query_compiler.to_pandas()) here with self._to_polars()?

modin/polars/base.py Outdated Show resolved Hide resolved

groupby = group_by

def drop(self, *columns):
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this is missing strict.

modin/polars/dataframe.py Show resolved Hide resolved
modin/polars/dataframe.py Show resolved Hide resolved
modin/polars/dataframe.py Show resolved Hide resolved
sfc-gh-dpetersohn and others added 3 commits July 3, 2024 09:58
Co-authored-by: Mahesh Vashishtha <[email protected]>
Signed-off-by: Devin Petersohn <[email protected]>
Signed-off-by: Devin Petersohn <[email protected]>
Copy link
Contributor

@sfc-gh-mvashishtha sfc-gh-mvashishtha left a comment

Choose a reason for hiding this comment

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

I've made it through len in groupby.py

Returns:
Arrow representation of the DataFrame.
"""
return polars.from_pandas(self._query_compiler.to_pandas()).to_arrow()
Copy link
Contributor

Choose a reason for hiding this comment

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

And should we replace all the polars.from_pandas(self._query_compiler.to_pandas()) here with self._to_polars()?

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.

Polars API
3 participants