Skip to content

Conversation

@rich-iannone
Copy link
Member

@rich-iannone rich-iannone commented Jun 15, 2021

This PR provides a bugfix, more flexibility, improved documentation, and more testing for the date/time formatting functions (fmt_date(), fmt_time(), and fmt_datetime()). Now Date and POSIXct columns are allowed to be formatted with these functions.

Fixes: #775
Fixes: #800
Fixes: #612

Copy link
Contributor

@krlmlr krlmlr left a comment

Choose a reason for hiding this comment

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

I had a very quick look. My biggest question here is: why does gt need to be concerned with parsing date/time/datetime values from strings? How bad would it be if gt accepted only properly typed data? Perhaps we could support two code paths, one for character input (legacy, using existing code) and one for typed input (without support for parsing)?

date_style %in% as.character(date_format_num_range)
) {
date_style <- as.numeric(date_style)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Nitpick: An else if here and below makes the purpose of these three blocks a bit clearer.

Suggested change
}
} else if ...

call. = FALSE
)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
} else if ...

time_style %in% as.character(time_format_num_range)
) {
time_style <- as.numeric(time_style)
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
} else if ...

call. = FALSE
)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
}
} else if ...

R/utils.R Outdated

has_dt_tz <- function(x) {

grepl(".+(Z|z|(\\+|-)[0-9]{2}([:]?[0-9]{2})?)?$", "2020-04-21T13:04:58.321Z")
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this used?

Suggested change
grepl(".+(Z|z|(\\+|-)[0-9]{2}([:]?[0-9]{2})?)?$", "2020-04-21T13:04:58.321Z")
grepl(".+(Z|z|(\\+|-)[0-9]{2}([:]?[0-9]{2})?)?$", x)

Copy link
Member Author

Choose a reason for hiding this comment

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

Now corrected. Currently it isn't used (should be removed before merging if never used).

rich-iannone and others added 10 commits January 21, 2022 10:46
* master:
  Fixes to footer (footnotes and source notes) across all output formats (#864)
  Row group labels as a column in stub (#855)
  Use reusable GHA workflows (#847)
  Increment version number
  v0.3.1 Release Candidate (#821)
  Fix for styles applied to summary cells when row groups are reordered (#814)
  Update image generation functions (#803)
  `cell_text()` fix (#805)
  Add whitespace options for text (#797)
@rich-iannone rich-iannone merged commit deeacdc into master Jan 28, 2022
@rich-iannone rich-iannone deleted the date-time-fmt-fix branch January 28, 2022 21:25
rich-iannone added a commit that referenced this pull request Jan 28, 2022
* master:
  Bug fixes and enhancements to date/time formatters (#801)
rich-iannone added a commit that referenced this pull request Jan 29, 2022
* master:
  Add ability to set global locale in `gt()` (#866)
  Bug fixes and enhancements to date/time formatters (#801)
rich-iannone added a commit that referenced this pull request Feb 1, 2022
* master:
  Add ability to set global locale in `gt()` (#866)
  Bug fixes and enhancements to date/time formatters (#801)
  Fixes to footer (footnotes and source notes) across all output formats (#864)
  Row group labels as a column in stub (#855)
  Use reusable GHA workflows (#847)
  Increment version number
  v0.3.1 Release Candidate (#821)
rich-iannone added a commit that referenced this pull request Feb 2, 2022
* master: (23 commits)
  Improve color handling and add tests (#870)
  Change HTML percent sign to literal `%` (#871)
  Add ability to set global locale in `gt()` (#866)
  Bug fixes and enhancements to date/time formatters (#801)
  Update gt_workflow_diagram.svg
  Update gt_parts_of_a_table.svg
  Update README.md
  Add dedicated ISSUE TEMPLATEs
  Update README.md
  Delete README.Rmd
  Update intro-creating-gt-tables.Rmd
  Update LICENSE
  Make corrections to docs (`columns`/`rows`)
  Update _pkgdown.yml
  Fixes to footer (footnotes and source notes) across all output formats (#864)
  Row group labels as a column in stub (#855)
  Use reusable GHA workflows (#847)
  Increment version number
  v0.3.1 Release Candidate (#821)
  Fix for styles applied to summary cells when row groups are reordered (#814)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

4 participants