Skip to content

dev UD - #28

Merged
Copilot merged 28 commits into
devfrom
main
Sep 1, 2026
Merged

Copilot merged 28 commits into
devfrom
main

Conversation

@vertesy

@vertesy vertesy commented Aug 28, 2026

Copy link
Copy Markdown
Owner

No description provided.

@vertesy

vertesy commented Aug 28, 2026

Copy link
Copy Markdown
Owner Author

@copilot
1 resolve the merge conflicts in this pull request
2 Go through the merge and make sure that we are not introducing any retrograde or regressive change. Also, make sure that we do not introduce problems that have been fixed on the development branch.

Copilot AI commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

@vertesy I can’t apply these changes because the pull request’s branch is the repository’s default branch, which I’m not allowed to push to directly.

You can ask me to create a new pull request with the requested changes instead and I'll open a separate PR on a new branch.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a6e50c876d

ℹ️ 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".

Comment thread R/ReadWriter.R
read_in <- as.data.frame(gtools::na.replace(read_in, replace = 0))
}

if (wRownames) read_in <- column.2.row.names(read_in, as_df = !asTibble)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Remove the undefined asTibble lookup

With the default wRownames = TRUE, every successful SSV read evaluates !asTibble, but read.simple.ssv() neither declares that parameter nor defines the variable. Normal callers therefore get object 'asTibble' not found instead of their data; add the parameter explicitly or retain the previous data-frame conversion behavior.

Useful? React with 👍 / 👎.

Comment thread R/ReadWriter.R
manual_directory = manual_directory
)

dir.create(dirname(FnP), recursive = TRUE, showWarnings = FALSE)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Create the target directory before validating it

When manual_directory names a directory that does not exist yet, construct.file.path() now stops at its dir.exists(out_dir) assertion, so execution can never reach this recursive dir.create() call. Create the directory before constructing the path, or allow the constructor to return paths under not-yet-created directories.

Useful? React with 👍 / 👎.

Comment thread R/ReadWriter.R Outdated
Comment on lines 717 to 719
write.simple.vec <- function(input_vec, filename = substitute(input_vec), suffix = NULL, extension = "vec",
manual_file_name = NULL, manual_directory = NULL, o = FALSE,
make_names = TRUE, manual_file_name = NULL, manual_directory = NULL, o = FALSE,
v = TRUE) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve positional order when adding make_names

Existing callers that pass manual_file_name as the fifth positional argument now bind that filename to make_names; the subsequent if (make_names) then receives a character value and errors. Add the new option after the existing parameters so the established positional API continues to work.

Useful? React with 👍 / 👎.

Comment thread R/ReadWriter.R Outdated
Comment on lines +936 to +937
o = FALSE, gzip = FALSE,
TabColor = "darkgoldenrod1", HeaderLineColor = "darkolivegreen3",

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Preserve positional order when adding gzip

For existing positional calls, the eighth argument previously configured TabColor but now binds to gzip, while every later styling argument is shifted as well. A color string ultimately reaches if (gzip) and causes an error after writing the workbook; append the new option after the existing arguments or otherwise preserve their positions.

Useful? React with 👍 / 👎.

@vertesy

vertesy commented Aug 31, 2026

Copy link
Copy Markdown
Owner Author

@claude

CODEX Have found the following issues with the merge. Could you please address them?

1
"Remove the undefined asTibble lookup

With the default wRownames = TRUE, every successful SSV read evaluates !asTibble, but read.simple.ssv() neither declares that parameter nor defines the variable. Normal callers therefore get object 'asTibble' not found instead of their data; add the parameter explicitly or retain the previous data-frame conversion behavior."

2
"Create the target directory before validating it

When manual_directory names a directory that does not exist yet, construct.file.path() now stops at its dir.exists(out_dir) assertion, so execution can never reach this recursive dir.create() call. Create the directory before constructing the path, or allow the constructor to return paths under not-yet-created directories."

3
"Preserve positional order when adding make_names

Existing callers that pass manual_file_name as the fifth positional argument now bind that filename to make_names; the subsequent if (make_names) then receives a character value and errors. Add the new option after the existing parameters so the established positional API continues to work."

4
"Preserve positional order when adding gzip

For existing positional calls, the eighth argument previously configured TabColor but now binds to gzip, while every later styling argument is shifted as well. A color string ultimately reaches if (gzip) and causes an error after writing the workbook; append the new option after the existing arguments or otherwise preserve their positions."

I Understand that this would mean editing the main branch, because we are updating the dev branch.

@claude

claude Bot commented Aug 31, 2026 •

Copy link
Copy Markdown

Claude finished @vertesy's task in 2m 48s —— View job


Todo list:

  • Gather context (read R/ReadWriter.R, PR diff vs origin/dev)
  • Fix To Do #1: undefined asTibble lookup in read.simple.ssv()
  • Fix Wiki #2: construct.file.path() validates manual_directory before it's created
  • Fix Resources #3: make_names param breaks positional args in write.simple.vec()
  • Fix Package errors #4: gzip param breaks positional args in write.simple.xlsx()
  • Sanity-checked diffs manually (see note below)
  • Commit and push to PR branch (main)

Summary

All 4 issues flagged by Codex have been fixed and pushed to main in commit b09a776 (R/ReadWriter.R + the corresponding .Rd files):

  1. read.simple.ssv() (R/ReadWriter.R:457-467) — added the missing asTibble = FALSE parameter (matching the sibling read.simple.tsv()/read.simple.csv() functions), fixing the object 'asTibble' not found error on every default (wRownames = TRUE) call.
  2. construct.file.path() (R/ReadWriter.R:198-199) — now creates manual_directory recursively (dir.create(out_dir, recursive = TRUE)) before the dir.exists() assertion, instead of only checking it. This lets write.simple.md.table() (and any other caller) target a not-yet-existing output directory.
  3. write.simple.vec() (R/ReadWriter.R:719-721) — moved the new make_names argument to the end of the parameter list (after v) so pre-existing positional calls (..., manual_file_name, manual_directory, o, v) keep binding to the correct parameters.
  4. write.simple.xlsx() (R/ReadWriter.R:930-944) — moved the new gzip argument to the end of the parameter list (after v) so pre-existing positional calls keep TabColor, HeaderLineColor, etc. bound to their original positions.

Updated the matching .Rd files (man/read.simple.ssv.Rd, man/write.simple.vec.Rd, man/write.simple.xlsx.Rd) by hand to stay in sync, since roxygen2/R is not installed in this sandbox and I could not regenerate docs automatically — please run devtools::document() locally to double check, though the manual edits mirror the source changes exactly.

Note on branch: Per your comment, I pushed directly to main, which is this PR's head branch (PR #28 is main → dev). The earlier Copilot agent declined because main is also the repo's default branch, but per your explicit confirmation this push was intentional and scoped only to these 4 fixes.

Note on testing: I was not able to run R CMD check, devtools::check(), or any R script in this sandbox (no R/Rscript binary available, and shell access outside the repo directory is blocked). The changes are minimal, targeted, and I manually traced each call site to confirm correctness — but please run the package check suite to confirm before merging into dev.
(branch: main)

1. read.simple.ssv(): declare the missing asTibble parameter so
   wRownames = TRUE (the default) no longer errors with
   "object 'asTibble' not found".
2. construct.file.path(): create manual_directory (recursively) before
   the dir.exists() assertion, so a not-yet-existing output directory
   no longer aborts before write.simple.md.table() (and other writers)
   can create it.
3. write.simple.vec(): move the new make_names argument to the end of
   the parameter list so old positional calls that pass
   manual_file_name as the 5th argument keep working.
4. write.simple.xlsx(): move the new gzip argument to the end of the
   parameter list so old positional calls keep TabColor and the other
   styling arguments bound to their original positions.

Co-authored-by: Abel Vertesy <5101911+vertesy@users.noreply.github.com>
Copilot AI merged commit e43ad13 into dev Sep 1, 2026
3 checks passed
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.

2 participants