-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix a bug in 'extract.timestamps' when dealing with an empty data source #270
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this PR @MaLoefUDS. I had a quick look at it, looks good to me. I just spotted a few minor issues, such as enhancing comments with additional information.
And it also seems that you need to rebase onto the current dev
branch.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## dev #270 +/- ##
==========================================
+ Coverage 80.92% 80.95% +0.03%
==========================================
Files 16 16
Lines 5043 5051 +8
==========================================
+ Hits 4081 4089 +8
Misses 962 962 ☔ View full report in Codecov by Sentry. |
Up until now, `get.data.cut.to.same.date(data.sources = c("issues", "mails", "commits"))` failed if some of the first data source was empty, but not if the second one was empty. The reason was that `NA` values introduced by empty data sources at the beginning of the data frame turned the data frame into a data frame of numeric objects instead of POSIXct objects. If there were already POSIXct objects in the data frame, this did not happen. To prevent the timestamps to be interpreted as numeric values, make sure that the `NA` values are always POSIXct objects. This fixes se-sic#269. Signed-off-by: Thomas Bock <[email protected]>
This test fails without the previous fix by Thomas Bock but does not fail when the fix is in place. This works towards fixing se-sic#269. Signed-off-by: Maximilian Löffler <[email protected]>
Signed-off-by: Maximilian Löffler <[email protected]>
*The last force push was just because of rebasing |
That's what I have thought anyway 😄 |
Signed-off-by: Maximilian Löffler <[email protected]>
Prerequisites
showcase.R
with respect to my changes.dev
.Description
Include a fix by @bockthom for issue #269 and a new unit test for the previously faulty behavior.
Changelog
Fixed:
extract.timestamps
that occurs when the firstdata.source
is empty and leads to a return value of typenumeric
which should bePOSIXct
.