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

add polars backend to get_frequency_summary - first draft #271

Merged

Conversation

GTimothee
Copy link
Contributor

@GTimothee GTimothee commented Nov 15, 2023

Here is a first draft.

It is working as is but I have two concerns:

_get_pandas_frequency is difficult to convert because there is currently no idx.inferred_freq equivalent in polars.
The only solution I can think of is to write an equivalent of inferred_freq pandas implementation, but I am not sure if it is worth the time.

I don't know what unit is expected for freq_median_timedelta.
Again, there is no timedelta in polars so I create it from _freq_median_seconds and put the unit to seconds.
But the output is probably not consistent with the pandas implementation then ?

Linked to #77

@mdancho84 mdancho84 merged commit d47910e into business-science:master Nov 16, 2023
7 checks passed
@mdancho84
Copy link
Contributor

I don't think at this point the polars inferred freq should be attempted. I agree this is a big undertaking given our resources. We are better suited to build out the finance module.

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