Repository navigation
Annotate code: explain the non-obvious operations - #30
Conversation
|
Understand this is an annotation request, but fix the following 3 bugs that have been identified:
|
|
Claude finished @vertesy's task in 2m 44s —— View job Tasks
SummaryFixed all 3 bugs you flagged, on top of the annotation-only diff, in commit
Not able to verify: R/Rscript isn't available in this sandbox, so I could not run The original comments-only annotation work from this PR is untouched; these are additive fixes layered on top per your explicit request in the trigger comment. |
- 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>
- 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>
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>
1ef3f08 to
db3d67d
Compare
* 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.
* 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>
* 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>
* 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>
* 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>
* 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>
* 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>
Summary
Adds a few targeted comments (the row-names reset trick in
column.2.row.names()), and normalizes two stray bare-string "comments" into real#/# TODO:comments.Bugs found while reading (not fixed, just flagged with
# NOTE:comments)read.simple.ssv(): referencesasTibble, which isn't a parameter of this function -- errors by default (wRownames = TRUE).write.simple.xlsx(): its internalassignRownameshelper usesdf(undefined) instead of its own argumentx-- errors wheneverhas_row_names = FALSE.qs.2.table(): broken throughout --qs:qread(single colon, should be::),stringi::stri_detectisn't a real function, andbase_filename/out_pathare used but never defined on most branches. This function doesn't currently work for any input.Flagging these for your call on whether/how to fix; didn't want to bundle behavior changes into a comments-only PR.