polars icon indicating copy to clipboard operation
polars copied to clipboard

feat(rust, python): add dt.quarter_start, dt.quarter_end, dt.year_start and dt.year_end

Open SozinM opened this issue 1 year ago β€’ 11 comments

closes #14511

  • [x] Implement functionality
  • [x] Implement tests
  • [x] Correct docs

SozinM avatar Feb 15 '24 18:02 SozinM

Codecov Report

Attention: Patch coverage is 96.48241% with 14 lines in your changes are missing coverage. Please review.

Project coverage is 81.28%. Comparing base (802d6c8) to head (e9d5f52).

:exclamation: Current head e9d5f52 differs from pull request most recent head 86a0755. Consider uploading reports for the commit 86a0755 to get more accurate results

Files Patch % Lines
crates/polars-time/src/period_start.rs 94.41% 10 Missing :warning:
...ates/polars-plan/src/dsl/function_expr/datetime.rs 92.45% 4 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #14513      +/-   ##
==========================================
+ Coverage   81.14%   81.28%   +0.13%     
==========================================
  Files        1362     1356       -6     
  Lines      174846   175938    +1092     
  Branches     2531     2522       -9     
==========================================
+ Hits       141881   143010    +1129     
+ Misses      32481    32444      -37     
  Partials      484      484              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Feb 29 '24 17:02 codecov[bot]

thanks - looks like it's on the right track, will do a full review next week

MarcoGorelli avatar Feb 29 '24 20:02 MarcoGorelli

All comments were addressed @MarcoGorelli

SozinM avatar Mar 11 '24 20:03 SozinM

thanks @SozinM for updating - could you update the merge conflict please? It should just be a matter of passing NonExistent::Raise to replace_time_zone

MarcoGorelli avatar Mar 19 '24 17:03 MarcoGorelli

@MarcoGorelli done :)

SozinM avatar Mar 22 '24 20:03 SozinM

@alexander-beedie Hi! Would you like to review this?

SozinM avatar Mar 26 '24 13:03 SozinM

@MarcoGorelli could you suggest a person to ask for an additional review?

SozinM avatar Mar 27 '24 11:03 SozinM

@alexander-beedie Hi! Would you like to review this?

~This is one for @MarcoGorelli πŸ˜ŽπŸ‘ But the functionality looks useful!~

Ahh, just spotted that it's already approved by @MarcoGorelli and you're just waiting for a merge. Looks like you need to rebase once more (there are currently some conflicts), and then we can take a final look and get it inπŸ‘Œ

alexander-beedie avatar Mar 28 '24 10:03 alexander-beedie

@alexander-beedie Hi! Rebased, take a look please :)

SozinM avatar Apr 02 '24 15:04 SozinM

@alexander-beedie soft bump

SozinM avatar Apr 16 '24 10:04 SozinM

Hi @c-peters and @stinodego, I randomly chose you to ask to take a look if you have free time :)

SozinM avatar Apr 22 '24 17:04 SozinM

Hi @reswqa , @orlp ! Maybe you will save the day? (or save this PR from being buried in history)

SozinM avatar May 02 '24 17:05 SozinM

Hi @ritchie46 maybe you will be able to help?

SozinM avatar May 10 '24 15:05 SozinM

hi @SozinM - seeing as this is still open, it may be good to get your thoughts on https://github.com/pola-rs/polars/issues/16094

if month_end were indeed disallowed for Datetimes, it may make sense to remove it from this PR too. do you have thoughts on that?

MarcoGorelli avatar May 10 '24 16:05 MarcoGorelli

I've felt a bit apprehensive pressing the merge button, because I'm not sure these methods are really that useful. Don't we already offer the right tools to do this yourself? E.g.

from datetime import date

import polars as pl

df = pl.DataFrame({"date": [date(2024, 2, 29)]})
result = df.with_columns(
    year_end=pl.date(year=pl.col("date").dt.year(), month=12, day=31)
)
print(result)
shape: (1, 2)
β”Œβ”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”¬β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”
β”‚ date       ┆ year_end   β”‚
β”‚ ---        ┆ ---        β”‚
β”‚ date       ┆ date       β”‚
β•žβ•β•β•β•β•β•β•β•β•β•β•β•β•ͺ════════════║
β”‚ 2024-02-29 ┆ 2024-12-31 β”‚
β””β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”΄β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”€β”˜

The API bloat just doesn't seem worth it.

stinodego avatar May 12 '24 17:05 stinodego

Okay, this PR got a little out of hand so I will close it. If somebody wants to pick it up - feel free to repurpose the code :)

SozinM avatar May 13 '24 15:05 SozinM