Skip to content

Conversation

shadnikn
Copy link

@shadnikn shadnikn commented Jun 4, 2025

@datapythonista datapythonista added the Benchmark Performance (ASV) benchmarks label Jun 4, 2025
@datapythonista
Copy link
Member

@rhshadrach do you have an opinion about this?


def time_plot_large_dataframe_single_column(self, size, datetime_index):
"""Baseline: plotting single column for comparison"""
self.df.iloc[:, 0].plot()
Copy link
Member

Choose a reason for hiding this comment

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

Can you do the iloc operation in setup so its not part of the timing?

Copy link
Author

Choose a reason for hiding this comment

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

Updated!

@shadnikn shadnikn force-pushed the ENH-61532-speedup-dataframe-plot branch from 54ee8b8 to 2324563 Compare June 11, 2025 02:15
@rhshadrach
Copy link
Member

@shadnikn - what is the runtime of these benchmarks on your machine?

Copy link
Member

@rhshadrach rhshadrach left a comment

Choose a reason for hiding this comment

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

Looks good, just some nit picks.

Comment on lines +170 to +171
[(1000, 10), (1000, 50), (1000, 100), (5000, 20), (10000, 10)],
[True, False] # DatetimeIndex or not
Copy link
Member

Choose a reason for hiding this comment

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

I think this is too many cases. Can you just do [(1000, 100)]



from .pandas_vb_common import setup # noqa: F401 isort:skip
class DataFramePlottingLarge:
Copy link
Member

Choose a reason for hiding this comment

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

I think what differentiates this from FramePlotting is that these are wide. Can you rename this class to FramePlottingWide.

[(1000, 10), (1000, 50), (1000, 100), (5000, 20), (10000, 10)],
[True, False] # DatetimeIndex or not
]
param_names = ["size", "datetime_index"]
Copy link
Member

Choose a reason for hiding this comment

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

Can you rename size -> shape.

@mroeschke
Copy link
Member

Thanks for the pull request, but it appears to have gone stale. If interested in continuing, please merge in the main branch, address any review comments and/or failing tests, and we can reopen.

@mroeschke mroeschke closed this Jul 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Benchmark Performance (ASV) benchmarks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants