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

[FEAT] 1606 - Adding hour expression in date util #1637

Merged
merged 2 commits into from
Nov 21, 2023
Merged

[FEAT] 1606 - Adding hour expression in date util #1637

merged 2 commits into from
Nov 21, 2023

Conversation

subygan
Copy link
Contributor

@subygan subygan commented Nov 20, 2023

Adds dt_hour() Expression to fetch hour of the current day in a column. Based on, #1606

Copy link

codecov bot commented Nov 20, 2023

Codecov Report

Merging #1637 (418dc1e) into main (c2d4c23) will increase coverage by 0.13%.
Report is 10 commits behind head on main.
The diff coverage is 75.00%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1637      +/-   ##
==========================================
+ Coverage   84.88%   85.02%   +0.13%     
==========================================
  Files          55       55              
  Lines        5287     5314      +27     
==========================================
+ Hits         4488     4518      +30     
+ Misses        799      796       -3     
Files Coverage Δ
daft/series.py 92.38% <100.00%> (+0.04%) ⬆️
daft/expressions/expressions.py 90.85% <50.00%> (-0.24%) ⬇️

... and 7 files with indirect coverage changes

@subygan subygan changed the title 1606 - Adding hour expression in date util FEAT 1606 - Adding hour expression in date util Nov 20, 2023
@samster25 samster25 added the enhancement New feature or request label Nov 21, 2023
@samster25 samster25 changed the title FEAT 1606 - Adding hour expression in date util [FEAT] 1606 - Adding hour expression in date util Nov 21, 2023
std::sync::Arc::new(Field::new(self.name(), DataType::UInt32)),
Box::new(date_arrow),
)
.unwrap_or(UInt32Array::from(("hour", vec![0]))))
Copy link
Member

Choose a reason for hiding this comment

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

You shouldn't have to unwrap the UInt32Array::new and then wrap it in Ok. You should be able to return the result from UInt32Array::new directly!

UInt32Array::new(
            std::sync::Arc::new(Field::new(self.name(), DataType::UInt32)),
            Box::new(date_arrow),
        )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, not sure what I was thinking. Fixing it now.

@samster25
Copy link
Member

looks great!

@samster25 samster25 merged commit 55a95ab into Eventual-Inc:main Nov 21, 2023
34 of 39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants