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

ud_convert has side effects #403

Closed
Enchufa2 opened this issue Mar 10, 2025 · 3 comments
Closed

ud_convert has side effects #403

Enchufa2 opened this issue Mar 10, 2025 · 3 comments
Labels

Comments

@Enchufa2
Copy link
Member

I always insist in my classes to avoid this and yet... :(

x <- 0.1
units::ud_convert(x, "m", "cm")
#> [1] 10
x
#> [1] 10
@Enchufa2 Enchufa2 added the bug label Mar 10, 2025
@Enchufa2
Copy link
Member Author

And not sure why, but this may have weird consequences when testthat is involved (see e.g. nlmixr2/babelmixr2/pull/153). It may be due to the magic they do to capture stuff and so on.

@edzer
Copy link
Member

edzer commented Mar 10, 2025

Oh lovely, we forgot to make a deep copy...

@Enchufa2
Copy link
Member Author

Oh lovely, we forgot to make a deep copy...

Exactly. It was an internal function, so we avoided making copies. But now that it's exported, we need to be more careful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants