Skip to content

Refactor/fixes - #515

Open
timcadman wants to merge 4 commits into
v7.0-devfrom
refactor/fixes
Open

timcadman wants to merge 4 commits into
v7.0-devfrom
refactor/fixes

Conversation

@timcadman

@timcadman timcadman commented Sep 22, 2026 •

Copy link
Copy Markdown
Contributor

Background

Fixes found in a full review of the v7.0 refactor (batches 1–10): a data leak, behaviour that changed relative to 6.3.5, and small doc and code errors.

What's changed

  • ds.asList data leak: asListDS returned the converted object to the client. It is now an assign function. It was an aggregate only to work around an old repository quirk with lists; tests confirm lists now assign correctly on Opal and Armadillo.
  • NULL objects: .loadServersideObject now errors on a NULL object ("is NULL") for every function, including ds.class, which used to return "NULL". Deliberate breaking change: NULL objects only arise from mistakes such as a typo in ds.make.
  • Class rules restored to 6.3.5 behaviour:
    • meanDS/varDS accept logical, so an all-NA column no longer breaks the call.
    • corDS/covDS accept factors again (correlated on level codes).
    • glm/glmSLMA/glmer/lmer/gamlss accept data.frame or list, not matrix, matching what the model functions accept.
    • lengthDS/dimDS accept any class.
    • meanSdGpDS accepts a numeric grouping variable.
  • Behaviour restored:
    • ds.cor/ds.cov ignore y (with a warning) when x is a matrix or data.frame.
    • ds.changeRefGroup treats a numeric ref as a level name again.
    • BooleDS accepts "TRUE"/"FALSE"/"NA" as V2.
    • levelsDS checks the privacy level first.
  • ds.mdPattern combine: pools columns by variable name, not position. md.pattern orders columns by missingness, which differs between studies, so variables could be mixed.
  • seqDS: from/to/by/length.out are converted with as.numeric instead of being looked up as object names.
  • glmChecks, ds.table: restore the user's warn option on exit. This fix was on 6.3.x but never ported to v7.

@timcadman
timcadman changed the base branch from v6.3.6-dev to v7.0-dev September 25, 2026 15:38
@timcadman
timcadman marked this pull request as ready for review September 25, 2026 15:44

@StuartWheater StuartWheater left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

  • Changes appear consistent
  • Test have corresponding updates
  • Docs have required updates

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants