Skip to content

Added dtt_excel_to_time() to convert excel times to R times #44

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Dec 2, 2024
Merged

Added dtt_excel_to_time() to convert excel times to R times #44

merged 8 commits into from
Dec 2, 2024

Conversation

dunkenwg
Copy link
Contributor

Also created tests for this function

@dunkenwg dunkenwg requested a review from aylapear October 11, 2024 18:32
aylapear
aylapear requested changes Oct 30, 2024
View reviewed changes
Copy link
Member

@aylapear aylapear left a comment

Choose a reason for hiding this comment

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

We should add a tests for empty numeric values. What happens if we pass numeric(0L) to the function, does it do what we expect?

The rest looks good otherwise

@aylapear aylapear self-assigned this Oct 30, 2024
@dunkenwg dunkenwg requested a review from aylapear October 31, 2024 16:21
@dunkenwg
Copy link
Contributor Author

@aylapear, does everything look good now?

aylapear
aylapear requested changes Nov 15, 2024
View reviewed changes
Copy link
Member

@aylapear aylapear 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 good. Do not merge the pull request but please do a dev version bump and update the news file.

dunkenwg and others added 2 commits November 15, 2024 16:04
@dunkenwg
Bump version to 0.5.2.9001
5804c31
@aylapear
merge fix
e5a6299 
Merge branch 'main' into exceltime

# Conflicts:
#	NEWS.md
@aylapear aylapear merged commit b1b843c into main Dec 2, 2024
1 check failed
@aylapear aylapear deleted the exceltime branch December 2, 2024 18:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aylapear aylapear aylapear approved these changes

Assignees

@aylapear aylapear

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@dunkenwg @aylapear