Conversation
…-documentation Polish function documentation
* 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>
# Conflicts: # DESCRIPTION # R/ReadWriter.R # R/list.of.functions.in.ReadWriter.md # README.md # man/column.2.row.names.Rd # man/read.simple.csv.named.vector.Rd # man/read.simple.ssv.Rd Co-authored-by: vertesy <5101911+vertesy@users.noreply.github.com>
…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>
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>
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>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 394b1a012f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| out_dir <- dirname(base_filename) | ||
| out_name <- basename(base_filename) |
There was a problem hiding this comment.
When out_file = "excel" and path is absolute, the newly computed out_dir and out_name are ignored by the Excel branch, which still passes the full base_filename as manual_file_name. Because write.simple.xlsx() leaves manual_directory unset, construct.file.path() prepends getwd(), so the write can target an invalid path and the reported out_path is not where the file was written. Pass these split components to the Excel writer as is done for the three text branches.
Useful? React with 👍 / 👎.
|
Please provide a proper solution in This PR, as a commit, which addresses the problem you raised. " Pass the split directory to the Excel writer When out_file = "excel" and path is absolute, the newly computed out_dir and out_name are ignored by the Excel branch, which still passes the full base_filename as manual_file_name. Because write.simple.xlsx() leaves manual_directory unset, construct.file.path() prepends getwd(), so the write can target an invalid path and the reported out_path is not where the file was written. Pass these split components to the Excel writer as is done for the three text branches." |
Summary
Testing
|
No description provided.