Skip to content

Conversation

@singjc
Copy link
Collaborator

@singjc singjc commented Nov 29, 2024

This is a draft PR to see how feasible it is to add support for polars dataframes. There are two approaches I can think of to do this.

  1. use the .to_pandas() method in polars to convert the polars dataframe to a pandas dataframe. This is the easiest, but requires the user to need both polars and pandas, but maybe that's okay.

  2. use a unified dataframe class that wraps either a pandas or polars dataframe, and mimics the behaviour of pandas, to avoid having to change all the internal plotting that uses pandas notation.

This PR is implementing the latter to see whether it's easy enough and feasible to do.

I am also testing registering a namespace to a polars dataframe, so we can do something like

import polars as pl
import pyopenms_viz 

df = pl.read_csv("mydata.csv")
df.mass.plot(kind="chromatogram", x="rt", y="int", by="ion")

However, this currently only registers the namespace and works if

# set global plotting backend for pandas
pd.options.plotting.backend = "ms_matplotlib" # one of: "ms_bokeh" "ms_matplotlib" "ms_plotly"

Not sure why, may be because this is how we define the entrypoint in our pyproject.toml.

^ Had to just add the import for the _dataframe module to the __init__.py file to register the namespace for polars

Any thoughts/comments/suggestions are welcome.

Comment on lines +125 to +128
@pl.api.register_dataframe_namespace("mass")
class UnifiedDataFrame:
"""
Wrapper class for Pandas and Polars DataFrames to provide a unified interface.

Choose a reason for hiding this comment

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

better. DataFrameInterface and Adapter instead of Wrapper

@timosachsenberg
Copy link

I like it.
One minor thing: maybe consider using names that match to common design pattern terminology.
Using DataFrameInterface and PandasDataFrameAdapter instead of e.g. wrapper

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