Skip to content

melt warns for measure.vars=list of length=1#6333

Merged
tdhock merged 6 commits intomasterfrom
melt-list-1
Aug 1, 2024
Merged

melt warns for measure.vars=list of length=1#6333
tdhock merged 6 commits intomasterfrom
melt-list-1

Conversation

@tdhock
Copy link
Member

@tdhock tdhock commented Aug 1, 2024

Closes #6071

This PR reverts the functionality introduced in #5247 and adds a new warning.

Old behavior is restored, to avoid a breaking change in the next release, which affected at least one revdep.

@tdhock tdhock requested a review from MichaelChirico as a code owner August 1, 2024 02:39
@github-actions
Copy link

github-actions bot commented Aug 1, 2024

Comparison Plot

Generated via commit e5b563f

Download link for the artifact containing the test results: ↓ atime-results.zip

Time taken to finish the standard R installation steps: 11 minutes and 35 seconds

Time taken to run atime::atime_pkg on the tests: 6 minutes and 15 seconds

@MichaelChirico
Copy link
Member

Thanks! Feel free to merge after addressing the comments.

tdhock and others added 5 commits August 1, 2024 09:11
Co-authored-by: Michael Chirico <michaelchirico4@gmail.com>
Co-authored-by: Michael Chirico <michaelchirico4@gmail.com>
@tdhock tdhock merged commit 2349536 into master Aug 1, 2024

## NOTES

7. `?melt` has long documented that the returned `variable` column should contain integer column indices when `measure.vars` is a list, but when the list length is 1, `variable` is actually a character column name, which is inconsistent with the documentation, [#5209](https://github.com/Rdatatable/data.table/issues/5209). To increase consistency in the next release, we plan to change `variable` to integer, so users who were relying on this behavior should change `measure.vars=list("col_name")` (output `variable` is column name, will be column index/integer) to `measure.vars="col_name"` (`variable` is column name before and after the planned change). For now, relying on this undocumented behavior throws a new warning.
Copy link
Member

Choose a reason for hiding this comment

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

whoops this retained the wrong number

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.

revdep vardpoor example failures after melt change

2 participants