Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Aug 1, 2024
Merged

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

merged 6 commits into from
Aug 1, 2024

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.

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

src/fmelt.c Outdated Show resolved Hide resolved
src/fmelt.c Outdated Show resolved Hide resolved
src/fmelt.c Outdated Show resolved Hide resolved
NEWS.md Outdated Show resolved Hide resolved
@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
5 checks passed
@@ -94,6 +92,8 @@

## 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