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

GH-34620: [C#] Support DateOnly and TimeOnly on .NET 6.0+ #36125

Merged
merged 6 commits into from
Jul 19, 2023

Conversation

CurtHagenlocher
Copy link
Contributor

@CurtHagenlocher CurtHagenlocher commented Jun 16, 2023

What changes are included in this PR?

Date32Array and Date64Array now support DateOnly values for construction and reading on .NET 6.0 and later.
Time32Array and Time64Array now support TimeOnly values for construction and reading on .NET 6.0 and later.
A new TimeArrayBuilder type is used to share logic between Time32Array.Builder and Time64Array.Builder just as the DateArrayBuilder does for the date array types.

Are these changes tested?

Yes

@github-actions
Copy link

⚠️ GitHub issue #34620 has been automatically assigned in GitHub to PR creator.

@github-actions
Copy link

⚠️ GitHub issue #34620 has been automatically assigned in GitHub to PR creator.

Copy link
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

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

This looks pretty solid. Just one question about the microsecond/nanosecond tick constants.

csharp/src/Apache.Arrow/Arrays/Date32Array.cs Show resolved Hide resolved
csharp/src/Apache.Arrow/Arrays/Time64Array.cs Outdated Show resolved Hide resolved
csharp/src/Apache.Arrow/Arrays/Time64Array.cs Show resolved Hide resolved
@github-actions github-actions bot added awaiting changes Awaiting changes and removed awaiting review Awaiting review labels Jul 17, 2023
@github-actions github-actions bot added awaiting change review Awaiting change review and removed awaiting changes Awaiting changes labels Jul 17, 2023
Copy link
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

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

Thanks for addressing the feedback.

@westonpace westonpace merged commit bebd2bf into apache:main Jul 19, 2023
9 checks passed
@westonpace westonpace removed the awaiting change review Awaiting change review label Jul 19, 2023
@github-actions github-actions bot added the awaiting merge Awaiting merge label Jul 19, 2023
chelseajonesr pushed a commit to chelseajonesr/arrow that referenced this pull request Jul 20, 2023
…he#36125)

### What changes are included in this PR?

Date32Array and Date64Array now support DateOnly values for construction and reading on .NET 6.0 and later.
Time32Array and Time64Array now support TimeOnly values for construction and reading on .NET 6.0 and later.
A new TimeArrayBuilder type is used to share logic between Time32Array.Builder and Time64Array.Builder just as the DateArrayBuilder does for the date array types.

### Are these changes tested?

Yes
* Closes: apache#34620

Authored-by: Curt Hagenlocher <[email protected]>
Signed-off-by: Weston Pace <[email protected]>
@CurtHagenlocher CurtHagenlocher deleted the DateOnly branch July 21, 2023 20:08
@conbench-apache-arrow
Copy link

After merging your PR, Conbench analyzed the 6 benchmarking runs that have been run so far on merge-commit bebd2bf.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details. It also includes information about possible false positives for unstable benchmarks that are known to sometimes produce them.

R-JunmingChen pushed a commit to R-JunmingChen/arrow that referenced this pull request Aug 20, 2023
…he#36125)

### What changes are included in this PR?

Date32Array and Date64Array now support DateOnly values for construction and reading on .NET 6.0 and later.
Time32Array and Time64Array now support TimeOnly values for construction and reading on .NET 6.0 and later.
A new TimeArrayBuilder type is used to share logic between Time32Array.Builder and Time64Array.Builder just as the DateArrayBuilder does for the date array types.

### Are these changes tested?

Yes
* Closes: apache#34620

Authored-by: Curt Hagenlocher <[email protected]>
Signed-off-by: Weston Pace <[email protected]>
loicalleyne pushed a commit to loicalleyne/arrow that referenced this pull request Nov 13, 2023
…he#36125)

### What changes are included in this PR?

Date32Array and Date64Array now support DateOnly values for construction and reading on .NET 6.0 and later.
Time32Array and Time64Array now support TimeOnly values for construction and reading on .NET 6.0 and later.
A new TimeArrayBuilder type is used to share logic between Time32Array.Builder and Time64Array.Builder just as the DateArrayBuilder does for the date array types.

### Are these changes tested?

Yes
* Closes: apache#34620

Authored-by: Curt Hagenlocher <[email protected]>
Signed-off-by: Weston Pace <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[C#] Support encoding DateOnly and TimeOnly values on .NET 6.
2 participants