Repository navigation
Improve rowname conversion and file path validation - #18
Merged
Merged
Conversation
vertesy
added a commit
that referenced
this pull request
Aug 12, 2025
vertesy
added a commit
that referenced
this pull request
Sep 1, 2026
* docs: remove duplicate title * Fix documentation typos and metadata (#17) * Add AGENTS guide for repository (#19) * Add defensive stopifnot checks to file path builder (#18) * Annotate code: explain the non-obvious operations (#30) * Annotate code; flag 3 pre-existing bugs (not fixed) * Fix 3 bugs flagged in PR #30 review - read.simple.ssv(): add missing `asTibble` parameter (was referenced but never defined, erroring whenever wRownames = TRUE, the default). - write.simple.xlsx(): assignRownames() now uses its own argument `x` instead of the undefined `df`, fixing has_row_names = FALSE. - qs.2.table(): fix `qs:qread` -> `qs::qread`, use the real `stringi::stri_detect_regex` function, and define `base_filename`/ `out_path` on every branch so the function actually runs. Also adds `stringi` to Imports/NAMESPACE since it's now genuinely used. * Fix qs.2.table()'s excel branch crashing on a bare data.frame write.simple.xlsx() requires a list of sheets; qs.2.table() was passing the loaded table directly, which fails its is.data.frame(x)-per-element assertion. Wrap it in a list first.
vertesy
added a commit
that referenced
this pull request
Sep 1, 2026
* docs: remove duplicate title * Fix documentation typos and metadata (#17) * Add AGENTS guide for repository (#19) * Add defensive stopifnot checks to file path builder (#18) * Annotate code: explain the non-obvious operations (#30) * Annotate code; flag 3 pre-existing bugs (not fixed) * Fix 3 bugs flagged in PR #30 review - read.simple.ssv(): add missing `asTibble` parameter (was referenced but never defined, erroring whenever wRownames = TRUE, the default). - write.simple.xlsx(): assignRownames() now uses its own argument `x` instead of the undefined `df`, fixing has_row_names = FALSE. - qs.2.table(): fix `qs:qread` -> `qs::qread`, use the real `stringi::stri_detect_regex` function, and define `base_filename`/ `out_path` on every branch so the function actually runs. Also adds `stringi` to Imports/NAMESPACE since it's now genuinely used. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> * Fix qs.2.table()'s excel branch crashing on a bare data.frame write.simple.xlsx() requires a list of sheets; qs.2.table() was passing the loaded table directly, which fails its is.data.frame(x)-per-element assertion. Wrap it in a list first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * Address Codex review: declare stringi dependency, preserve row names on xlsx export - DESCRIPTION: add stringi to Imports, since R/ReadWriter.R now calls stringi::stri_detect_regex() and NAMESPACE already declares the importFrom. - write.simple.xlsx(): when has_row_names = FALSE, the identifier column is moved into row names via column.2.row.names() before writing. The final write.xlsx() call still passed the original has_row_names (FALSE) as rowNames, so those converted identifiers were dropped from the workbook. Track the post-conversion state separately (write_row_names) and pass that to write.xlsx() instead. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
vertesy
added a commit
that referenced
this pull request
Sep 1, 2026
* docs: remove duplicate title * Fix documentation typos and metadata (#17) * Add AGENTS guide for repository (#19) * Add defensive stopifnot checks to file path builder (#18) * Annotate code: explain the non-obvious operations (#30) * Annotate code; flag 3 pre-existing bugs (not fixed) * Fix 3 bugs flagged in PR #30 review - read.simple.ssv(): add missing `asTibble` parameter (was referenced but never defined, erroring whenever wRownames = TRUE, the default). - write.simple.xlsx(): assignRownames() now uses its own argument `x` instead of the undefined `df`, fixing has_row_names = FALSE. - qs.2.table(): fix `qs:qread` -> `qs::qread`, use the real `stringi::stri_detect_regex` function, and define `base_filename`/ `out_path` on every branch so the function actually runs. Also adds `stringi` to Imports/NAMESPACE since it's now genuinely used. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> * Fix qs.2.table()'s excel branch crashing on a bare data.frame write.simple.xlsx() requires a list of sheets; qs.2.table() was passing the loaded table directly, which fails its is.data.frame(x)-per-element assertion. Wrap it in a list first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * Address Codex review: declare stringi dependency, preserve row names on xlsx export - DESCRIPTION: add stringi to Imports, since R/ReadWriter.R now calls stringi::stri_detect_regex() and NAMESPACE already declares the importFrom. - write.simple.xlsx(): when has_row_names = FALSE, the identifier column is moved into row names via column.2.row.names() before writing. The final write.xlsx() call still passed the original has_row_names (FALSE) as rowNames, so those converted identifiers were dropped from the workbook. Track the post-conversion state separately (write_row_names) and pass that to write.xlsx() instead. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
vertesy
added a commit
that referenced
this pull request
Sep 1, 2026
* docs: remove duplicate title * Fix documentation typos and metadata (#17) * Add AGENTS guide for repository (#19) * Add defensive stopifnot checks to file path builder (#18) * Annotate code: explain the non-obvious operations (#30) * Annotate code; flag 3 pre-existing bugs (not fixed) * Fix 3 bugs flagged in PR #30 review - read.simple.ssv(): add missing `asTibble` parameter (was referenced but never defined, erroring whenever wRownames = TRUE, the default). - write.simple.xlsx(): assignRownames() now uses its own argument `x` instead of the undefined `df`, fixing has_row_names = FALSE. - qs.2.table(): fix `qs:qread` -> `qs::qread`, use the real `stringi::stri_detect_regex` function, and define `base_filename`/ `out_path` on every branch so the function actually runs. Also adds `stringi` to Imports/NAMESPACE since it's now genuinely used. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> * Fix qs.2.table()'s excel branch crashing on a bare data.frame write.simple.xlsx() requires a list of sheets; qs.2.table() was passing the loaded table directly, which fails its is.data.frame(x)-per-element assertion. Wrap it in a list first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * Address Codex review: declare stringi dependency, preserve row names on xlsx export - DESCRIPTION: add stringi to Imports, since R/ReadWriter.R now calls stringi::stri_detect_regex() and NAMESPACE already declares the importFrom. - write.simple.xlsx(): when has_row_names = FALSE, the identifier column is moved into row names via column.2.row.names() before writing. The final write.xlsx() call still passed the original has_row_names (FALSE) as rowNames, so those converted identifiers were dropped from the workbook. Track the post-conversion state separately (write_row_names) and pass that to write.xlsx() instead. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> * Consolidate list-of-functions cleanup into one file.remove() call (#34) Replaces the per-file file.remove() call with a single call that matches every generated list.of.functions.in.*.det.md report by pattern (this repo generates 2: ReadWriter and Deprecated.Functions -- only the first was ever cleaned up before). The old line is commented out, not deleted. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
vertesy
added a commit
that referenced
this pull request
Sep 2, 2026
* docs: remove duplicate title * Fix documentation typos and metadata (#17) * Add AGENTS guide for repository (#19) * Add defensive stopifnot checks to file path builder (#18) * Annotate code: explain the non-obvious operations (#30) * Annotate code; flag 3 pre-existing bugs (not fixed) * Fix 3 bugs flagged in PR #30 review - read.simple.ssv(): add missing `asTibble` parameter (was referenced but never defined, erroring whenever wRownames = TRUE, the default). - write.simple.xlsx(): assignRownames() now uses its own argument `x` instead of the undefined `df`, fixing has_row_names = FALSE. - qs.2.table(): fix `qs:qread` -> `qs::qread`, use the real `stringi::stri_detect_regex` function, and define `base_filename`/ `out_path` on every branch so the function actually runs. Also adds `stringi` to Imports/NAMESPACE since it's now genuinely used. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> * Fix qs.2.table()'s excel branch crashing on a bare data.frame write.simple.xlsx() requires a list of sheets; qs.2.table() was passing the loaded table directly, which fails its is.data.frame(x)-per-element assertion. Wrap it in a list first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * Address Codex review: declare stringi dependency, preserve row names on xlsx export - DESCRIPTION: add stringi to Imports, since R/ReadWriter.R now calls stringi::stri_detect_regex() and NAMESPACE already declares the importFrom. - write.simple.xlsx(): when has_row_names = FALSE, the identifier column is moved into row names via column.2.row.names() before writing. The final write.xlsx() call still passed the original has_row_names (FALSE) as rowNames, so those converted identifiers were dropped from the workbook. Track the post-conversion state separately (write_row_names) and pass that to write.xlsx() instead. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> * Consolidate list-of-functions cleanup into one file.remove() call (#34) Replaces the per-file file.remove() call with a single call that matches every generated list.of.functions.in.*.det.md report by pattern (this repo generates 2: ReadWriter and Deprecated.Functions -- only the first was ever cleaned up before). The old line is commented out, not deleted. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
vertesy
added a commit
that referenced
this pull request
Sep 2, 2026
* docs: remove duplicate title * Fix documentation typos and metadata (#17) * Add AGENTS guide for repository (#19) * Add defensive stopifnot checks to file path builder (#18) * Annotate code: explain the non-obvious operations (#30) * Annotate code; flag 3 pre-existing bugs (not fixed) * Fix 3 bugs flagged in PR #30 review - read.simple.ssv(): add missing `asTibble` parameter (was referenced but never defined, erroring whenever wRownames = TRUE, the default). - write.simple.xlsx(): assignRownames() now uses its own argument `x` instead of the undefined `df`, fixing has_row_names = FALSE. - qs.2.table(): fix `qs:qread` -> `qs::qread`, use the real `stringi::stri_detect_regex` function, and define `base_filename`/ `out_path` on every branch so the function actually runs. Also adds `stringi` to Imports/NAMESPACE since it's now genuinely used. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> * Fix qs.2.table()'s excel branch crashing on a bare data.frame write.simple.xlsx() requires a list of sheets; qs.2.table() was passing the loaded table directly, which fails its is.data.frame(x)-per-element assertion. Wrap it in a list first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * Address Codex review: declare stringi dependency, preserve row names on xlsx export - DESCRIPTION: add stringi to Imports, since R/ReadWriter.R now calls stringi::stri_detect_regex() and NAMESPACE already declares the importFrom. - write.simple.xlsx(): when has_row_names = FALSE, the identifier column is moved into row names via column.2.row.names() before writing. The final write.xlsx() call still passed the original has_row_names (FALSE) as rowNames, so those converted identifiers were dropped from the workbook. Track the post-conversion state separately (write_row_names) and pass that to write.xlsx() instead. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> * Consolidate list-of-functions cleanup into one file.remove() call (#34) Replaces the per-file file.remove() call with a single call that matches every generated list.of.functions.in.*.det.md report by pattern (this repo generates 2: ReadWriter and Deprecated.Functions -- only the first was ever cleaned up before). The old line is commented out, not deleted. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * Fix qs.2.table to preserve directory of absolute input paths Pass dirname(path) as manual_directory and the extensionless basename(path) as manual_file_name for every output branch, so construct.file.path() no longer prepends getwd() to an already absolute filename (Codex review feedback on #38). Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
vertesy
added a commit
that referenced
this pull request
Sep 2, 2026
* docs: remove duplicate title * Fix documentation typos and metadata (#17) * Add AGENTS guide for repository (#19) * Add defensive stopifnot checks to file path builder (#18) * Annotate code: explain the non-obvious operations (#30) * Annotate code; flag 3 pre-existing bugs (not fixed) * Fix 3 bugs flagged in PR #30 review - read.simple.ssv(): add missing `asTibble` parameter (was referenced but never defined, erroring whenever wRownames = TRUE, the default). - write.simple.xlsx(): assignRownames() now uses its own argument `x` instead of the undefined `df`, fixing has_row_names = FALSE. - qs.2.table(): fix `qs:qread` -> `qs::qread`, use the real `stringi::stri_detect_regex` function, and define `base_filename`/ `out_path` on every branch so the function actually runs. Also adds `stringi` to Imports/NAMESPACE since it's now genuinely used. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> * Fix qs.2.table()'s excel branch crashing on a bare data.frame write.simple.xlsx() requires a list of sheets; qs.2.table() was passing the loaded table directly, which fails its is.data.frame(x)-per-element assertion. Wrap it in a list first. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * Address Codex review: declare stringi dependency, preserve row names on xlsx export - DESCRIPTION: add stringi to Imports, since R/ReadWriter.R now calls stringi::stri_detect_regex() and NAMESPACE already declares the importFrom. - write.simple.xlsx(): when has_row_names = FALSE, the identifier column is moved into row names via column.2.row.names() before writing. The final write.xlsx() call still passed the original has_row_names (FALSE) as rowNames, so those converted identifiers were dropped from the workbook. Track the post-conversion state separately (write_row_names) and pass that to write.xlsx() instead. Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> * Consolidate list-of-functions cleanup into one file.remove() call (#34) Replaces the per-file file.remove() call with a single call that matches every generated list.of.functions.in.*.det.md report by pattern (this repo generates 2: ReadWriter and Deprecated.Functions -- only the first was ever cleaned up before). The old line is commented out, not deleted. Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * Fix qs.2.table to preserve directory of absolute input paths Pass dirname(path) as manual_directory and the extensionless basename(path) as manual_file_name for every output branch, so construct.file.path() no longer prepends getwd() to an already absolute filename (Codex review feedback on #38). Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> * Fix Excel output path for QS conversion --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
column.2.row.namesto accept column names or indices and warn on duplicates using actual valuesconstruct.file.pathrequires filename information, validates arguments viastopifnot, and returns a valid pathTesting
R -q -e "print('test')"(fails: command not found)https://chatgpt.com/codex/tasks/task_e_68931e2e6b90832c98570167745a2a98