Skip to content
This repository has been archived by the owner on Aug 23, 2022. It is now read-only.

Time bug fix #165

Closed
wants to merge 3 commits into from
Closed

Time bug fix #165

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4a3904b
Fix for vector memory allocation error
DocEd Jul 27, 2018
98a7f76
gitignore update
DocEd Jul 27, 2018
0ade6a3
Minor update to account for NA max time
DocEd Jul 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions .Rbuildignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,5 @@ Makefile
.travis.yml
appveyor.yml
paper.md
^.*\.Rproj$
^\.Rproj\.user$
Copy link
Collaborator

Choose a reason for hiding this comment

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

You don't need to add these to the PR. It should just be the things needed to fix the issue you are worried about.

2 changes: 2 additions & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,5 @@ vignettes/*.pdf

# OAuth2 token, see https://github.com/hadley/httr/releases/tag/v0.3
*.httr-oauth
.Rproj.user
*.Rproj
Copy link
Collaborator

Choose a reason for hiding this comment

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

You wouldn't normally add these to the git repo either. There is a tendency to put IDE / editor specific ignore stuff in your system gitignore to prevent it from getting too crazy. That said, this has the same, you probably wouldn't add it to a PR as it isn't relevant property as above.

16 changes: 13 additions & 3 deletions R/ccTable.R
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -452,9 +452,17 @@ getEpisodePeriod <- function (e, unit="hours") {
# The failure of POSIX conversion indicates that this episode is either
# anonymised or has a missing or incorrect value of discharge or admission
# time.
if (is.na(tadm) || is.na(tdisc))
period_length <- findMaxTime(e)
else {
if (is.na(tdisc)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

tadm can probably be NA in general. It is only a specific property of the dataset you have that it cannot.

if (is.null(findMaxTime(e))) {
period_length <- 0
} else {
period_length <- ceiling(
as.numeric(
difftime(
as.POSIXct.numeric(findMaxTime(e), origin = "1970-01-01 00:00:00"), tadm, units = unit)
))
}
} else {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are you finding max time twice?

if (any(is.null(tdisc), is.null(tadm)))
period_length <- NULL
else
Expand All @@ -466,6 +474,8 @@ getEpisodePeriod <- function (e, unit="hours") {
if (period_length == 0)
period_length <- period_length + 1
}


Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are you adding two blank lines here? What are they for?


if (is.null(period_length))
warning("This episode does not have any time series data: ",
Expand Down