Skip to content

Fix datetime printing issue#134

Merged
gowerc merged 3 commits into
masterfrom
132-fix-ascii-table
Aug 27, 2025
Merged

Fix datetime printing issue#134
gowerc merged 3 commits into
masterfrom
132-fix-ascii-table

Conversation

@gowerc

@gowerc gowerc commented Aug 9, 2025

Copy link
Copy Markdown
Owner

Closes #132

Comment thread R/ascii_tables.R
#' @export
as_fmt_char.POSIXt <- function(x, ...) {
format(x, "%Y-%m-%d %H:%M:%S %Z")
x <- format(x, "%Y-%m-%d %H:%M:%S %Z")

@gowerc gowerc Aug 9, 2025

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I think from memory this code exists (as opposed to just using as.character() ) to force the timestamp to be shown. Regular dates already have the NA conversion covered within the default as.character() handler but for some reason that was missed here

@github-actions

github-actions Bot commented Aug 9, 2025

Copy link
Copy Markdown
Contributor

badge

Code Coverage Summary

Filename                Stmts    Miss  Cover    Missing
--------------------  -------  ------  -------  --------------------------
R/ascii_tables.R          117       8  93.16%   10, 153, 164, 169-172, 240
R/cast_variables.R         49       0  100.00%
R/diffdf.R                209      18  91.39%   373-390, 417
R/generate_keyname.R       12       0  100.00%
R/identify.R              152       8  94.74%   283-290
R/is_different.R           52       0  100.00%
R/issuerows.R              40       0  100.00%
R/issues.R                 17       1  94.12%   52
R/misc_functions.R         34       2  94.12%   9, 13
R/print.R                  20       0  100.00%
TOTAL                     702      37  94.73%

Results for commit: f9f0c06

Minimum allowed coverage is 80%

♻️ This comment has been updated with latest results

@github-actions

github-actions Bot commented Aug 9, 2025

Copy link
Copy Markdown
Contributor

Unit Tests Summary

  1 files   13 suites   7s ⏱️
 57 tests  53 ✅  4 💤 0 ❌
591 runs  580 ✅ 11 💤 0 ❌

Results for commit f9f0c06.

♻️ This comment has been updated with latest results.

@bundfussr

Copy link
Copy Markdown

Hi @gowerc , @kieranjmartin , when do you plan to merge and release the bug fix?

The bug prevents displaying the differences in the admiralroche CI/CD jobs.

@gowerc

gowerc commented Aug 21, 2025

Copy link
Copy Markdown
Owner Author

I think Kieran is back off holiday next week so this should be merged then, I was also hoping to resolve #135 & #133 and then release it as a bug patch, most of those seem pretty simple to fix so maybe end of next week for release (or start of week after if anything comes up).

@bundfussr

Copy link
Copy Markdown

I think Kieran is back off holiday next week so this should be merged then, I was also hoping to resolve #135 & #133 and then release it as a bug patch, most of those seem pretty simple to fix so maybe end of next week for release (or start of week after if anything comes up).

Sounds good. Thanks.

@kieranjmartin kieranjmartin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

minor changes, just to expand the testing a little to ensure we have coverage

Comment thread tests/testthat/test-miscellaneous.R Outdated
Comment thread tests/testthat/test-miscellaneous.R
@gowerc gowerc requested a review from kieranjmartin August 26, 2025 19:04
@gowerc

gowerc commented Aug 26, 2025

Copy link
Copy Markdown
Owner Author

@kieranjmartin - Should be good for re-review

@gowerc gowerc merged commit 389c785 into master Aug 27, 2025
23 checks passed
@gowerc gowerc deleted the 132-fix-ascii-table branch August 27, 2025 14:50
@gowerc

gowerc commented Oct 19, 2025

Copy link
Copy Markdown
Owner Author

@bundfussr - This is now on CRAN

@bundfussr

Copy link
Copy Markdown

@bundfussr - This is now on CRAN

Great, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

as_ascii table does not handle NA values in some cases.

3 participants