-
Notifications
You must be signed in to change notification settings - Fork 17
Closes #117 Update for ignore_seconds_flag #118
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
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.
Pull Request Overview
This pull request adds the ignore_seconds_flag = FALSE parameter to derive_vars_dtm() function calls in pharmacokinetic analysis scripts to address issue #117.
Key Changes:
- Added explicit
ignore_seconds_flag = FALSEparameter to PC data derivation calls in ADPC and ADPPK datasets - Added trailing blank lines to R script files for better formatting
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| adam/adpc.R | Added ignore_seconds_flag = FALSE to PC date derivation and trailing blank line |
| adam/adpc.qmd | Added ignore_seconds_flag = FALSE to PC date derivation in Quarto document |
| adam/adppk.R | Added ignore_seconds_flag = FALSE to PC date derivation and trailing blank line |
| adam/adppk.qmd | Added ignore_seconds_flag = FALSE to PC date derivation in Quarto document |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| time_imputation = "00:00:00", | ||
| ignore_seconds_flag = FALSE |
Copilot
AI
Nov 20, 2025
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.
The ignore_seconds_flag = FALSE parameter is added to the derive_vars_dtm() call for PC data, but the same parameter is not added to the derive_vars_dtm() calls for EX data (lines 132-135 and 137-140). For consistency, consider adding ignore_seconds_flag = FALSE to all derive_vars_dtm() calls in this file, or provide a comment explaining why only the PC data derivation requires this parameter.
| time_imputation = "00:00:00", | ||
| ignore_seconds_flag = FALSE |
Copilot
AI
Nov 20, 2025
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.
The ignore_seconds_flag = FALSE parameter is added to the derive_vars_dtm() call for PC data, but the same parameter is not added to the derive_vars_dtm() calls for EX data (lines 81-84 and 86-89). For consistency, consider adding ignore_seconds_flag = FALSE to all derive_vars_dtm() calls in this file, or provide a comment explaining why only the PC data derivation requires this parameter.
| time_imputation = "00:00:00", | ||
| ignore_seconds_flag = FALSE |
Copilot
AI
Nov 20, 2025
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.
The ignore_seconds_flag = FALSE parameter is added to the derive_vars_dtm() call for PC data, but the same parameter is not added to the derive_vars_dtm() calls for EX data (lines 136-139 and 141-144). For consistency, consider adding ignore_seconds_flag = FALSE to all derive_vars_dtm() calls in this file, or provide a comment explaining why only the PC data derivation requires this parameter.
| time_imputation = "00:00:00", | ||
| ignore_seconds_flag = FALSE |
Copilot
AI
Nov 20, 2025
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.
The ignore_seconds_flag = FALSE parameter is added to the derive_vars_dtm() call for PC data, but the same parameter is not added to the derive_vars_dtm() calls for EX data (lines 76-79 and 81-84). For consistency, consider adding ignore_seconds_flag = FALSE to all derive_vars_dtm() calls in this file, or provide a comment explaining why only the PC data derivation requires this parameter.
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.
oh - we don't need to do anything with EX right?
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.
Right, but only because I know EX doesn't have seconds, but PC does
Pull Request
DESCRIPTION GOES HERE
Before you submit your pull request, take a look at the following checklist. Many thanks for your contribution!
Closes #<insert_issue_number>at the beginning of your PR title. Use the Edit button in the top-right if you need to update.DESCRIPTIONfile.DESCRIPTIONfile'sImportssection.