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

fix(rust): typo in add_half_life takes ln(negative) #15932

Merged
merged 1 commit into from
Apr 28, 2024

Conversation

jr200
Copy link
Contributor

@jr200 jr200 commented Apr 27, 2024

Updated fn body to match documented formula.

@github-actions github-actions bot added fix Bug fix rust Related to Rust Polars labels Apr 27, 2024
@MarcoGorelli
Copy link
Collaborator

Thanks - your fix looks correct, I'm just confused by this function. It's not used anywhere? It's not tested? Maybe it should just be removed?

@jr200
Copy link
Contributor Author

jr200 commented Apr 27, 2024

Thanks - your fix looks correct, I'm just confused by this function. It's not used anywhere? It's not tested? Maybe it should just be removed?

Yes I noticed that - its also under src/legacy, which sounds like its on its way out. Better to delete it if we don't want people to construct EWMOptions using those methods I guess...

@stinodego
Copy link
Member

stinodego commented Apr 28, 2024

Thanks - your fix looks correct, I'm just confused by this function. It's not used anywhere? It's not tested? Maybe it should just be removed?

I believe it's part of the Rust public API. We might remove it at some point, but until we do, it better be correct.

@ritchie46 ritchie46 merged commit 14b352f into pola-rs:main Apr 28, 2024
20 of 22 checks passed
Wouittone pushed a commit to Wouittone/polars that referenced this pull request Jun 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants