-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat(python): Added support for pickling lace.Engine #184
feat(python): Added support for pickling lace.Engine #184
Conversation
fde6635
to
6ccbed7
Compare
6ccbed7
to
b29232b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One question, but not enough to hold stuff up. LGTM!
pylace/src/lib.rs
Outdated
polars::df! { | ||
"ID" => [0], | ||
} | ||
.expect("Should be a df"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do an Ok(thing.expect("")
instead of just returning the Result
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm honestly surprised clippy
didn't notice it
sim_a = engine.simulate(["swims", "flys"], n=10) | ||
sim_b = engine_b.simulate(["swims", "flys"], n=10) | ||
|
||
assert sim_a.equals(sim_b) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good practice to add a test, I've been lagging in that lately
No description provided.