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

updated documentation regarding behavior of rbindlist when applied to difftime objects with different units #6309

Merged
merged 4 commits into from
Jul 29, 2024

Conversation

Nj221102
Copy link
Contributor

@Nj221102 Nj221102 commented Jul 23, 2024

closes #4541

Summary

The current implementation of rbindlist does not preserve the units of difftime objects correctly when combining data.table or data.frame objects with different units. Instead, it converts all difftime values to a common unit without adjusting the values themselves. This can lead to incorrect time differences in the resulting data.table.

This pull request proposes updating the documentation to inform users that they need to manually ensure consistent units for difftime objects before using rbindlist to avoid inaccuracies.

@Nj221102 Nj221102 requested review from tdhock and Anirban166 July 23, 2024 11:29
@Nj221102 Nj221102 requested a review from MichaelChirico as a code owner July 23, 2024 11:29
@MichaelChirico MichaelChirico requested review from ben-schwen and removed request for MichaelChirico July 23, 2024 14:46
@ben-schwen
Copy link
Member

As pointed out by Jan in the issue I don't think that a tailored solution to a certain class is a favorable solution. Adding documentation about a single class is therefore not ideal either.

Can we generalize this to classes where the unit depends on class attributes?

@MichaelChirico
Copy link
Member

I agree about the need to be more generic here.

@Nj221102
Copy link
Contributor Author

@ben-schwen is this good enough ??

When binding lists of \code{data.table} or \code{data.frame} objects containing objects with units defined by class attributes (e.g., \code{difftime} objects with different units), the resulting \code{data.table} may not preserve the original units correctly. Instead, values will be converted to a common unit (typically the smallest unit present in the input) without proper conversion of the values themselves. This issue applies to any class where the unit or precision is determined by attributes. Users should manually ensure that objects with unit-dependent attributes have consistent units before using \code{rbindlist}

man/rbindlist.Rd Outdated Show resolved Hide resolved
@Nj221102 Nj221102 requested a review from ben-schwen July 29, 2024 15:13
man/rbindlist.Rd Outdated Show resolved Hide resolved
@ben-schwen ben-schwen merged commit b3c24a6 into master Jul 29, 2024
4 checks passed
@ben-schwen ben-schwen deleted the rbind branch July 29, 2024 19:40
@ben-schwen
Copy link
Member

Ty Nitish!

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.

rbindlist gives wrong results when it is applied to difftime objects with different units.
3 participants