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

[NEW] month_delta function #57

Merged
merged 12 commits into from
Mar 9, 2024
Merged

[NEW] month_delta function #57

merged 12 commits into from
Mar 9, 2024

Conversation

akmalsoliev
Copy link
Contributor

The month_diff function computes the integer representation of the month difference between two dates. The resulting values can be both positive and negative, signifying whether the comparative shift from the target date is to the past or the future, respectively.

src/month_delta.rs Outdated Show resolved Hide resolved
polars_xdt/__init__.py Outdated Show resolved Hide resolved
src/month_delta.rs Outdated Show resolved Hide resolved
@akmalsoliev akmalsoliev changed the title [NEW] month_diff function [NEW] month_delta function Jan 21, 2024
@akmalsoliev
Copy link
Contributor Author

akmalsoliev commented Jan 22, 2024

Work In Progress

Issues:

  • Border dates are still not working properly.
  • 31st of January - 28th of February is still not considered as one whole month.

@MarcoGorelli
Copy link
Collaborator

31st of January - 28th of February is still not considered as one whole month.

this looks fine to be honest

say you want to count the whole months between x and y, where y > x

the result is the smallest n such that x.dt.offset_by(f'{n}mo') is greater or equal to y

src/month_delta.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

is this correct? shouldn't it be 11?

(Pdb) pl.select(xdt.month_delta(dt.date(1970, 1, 2), dt.date(1971, 1, 1)))
shape: (1, 1)
┌─────────┐
│ literal │
│ ---     │
│ i32     │
╞═════════╡
│ 12      │
└─────────┘

@akmalsoliev
Copy link
Contributor Author

akmalsoliev commented Feb 8, 2024

is this correct? shouldn't it be 11?

(Pdb) pl.select(xdt.month_delta(dt.date(1970, 1, 2), dt.date(1971, 1, 1)))
shape: (1, 1)
┌─────────┐
│ literal │
│ ---     │
│ i32     │
╞═════════╡
│ 12      │
└─────────┘

Fixed, there was unnecessary month comparison subtraction_condition that has there from previous commits.

NOTE: Added this to test section too.

@MarcoGorelli
Copy link
Collaborator

thanks for updating!

OK I think the results are correct. Or at least, I wasn't able to break it, even with hypothesis :)

I'll do another check of the code, and of the performance, as I'd like to think this can be simplified

Copy link
Collaborator

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

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

thanks @akmalsoliev !

@MarcoGorelli MarcoGorelli merged commit 1f39027 into pola-rs:main Mar 9, 2024
14 checks passed
@MarcoGorelli
Copy link
Collaborator

#66 just fyi (now that we've got a hypothesis test in, we can confidently optimise away 😎 )

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