-
Notifications
You must be signed in to change notification settings - Fork 38
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
Implement $name$prefix_fields()
and $name$suffix_fields()
#873
Conversation
# TODO: this works but always prints the error message of log("a"), how | ||
# can we silence it? | ||
# expect_error( | ||
# pl$DataFrame(list(alice=1:3))$select( | ||
# pl$col("alice")$name$map(\(x) log("a")) | ||
# ) | ||
# ) |
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.
How about using suppressWarnings
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.
It doesn't work, log("a")
generates an error, not a warning. I'd say we can skip this for now, there's another test above for wrong function output
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.
Hmmm, I don't know what this test case is trying to test.
log()
is an R function, not a polars one, right?
If it doesn't make sense, why not simply delete the comment?
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.
We want to test 2 types of errors:
- the function called in
$map()
works but doesn't return a string -> Polars error - the function called in
$map()
fails (likelog("a")
) -> both R and Polars error, and we want to ensure that it doesn't lead to a panick or another issue with threads
The test that is commented out is for the second case. Problem is that it returns the two errors and expect_error()
only hides the Polars one, not the R one. Wrapping expect_error()
in a second expect_error()
doesn't work either.
To clarify, if we uncomment this test, it correctly passes. The only issue is that it prints the error message of log("a")
in the output
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.
Thanks for the explanation. Okay, I think we can merge it now for now.
Co-authored-by: eitsupi <50911393+eitsupi@users.noreply.github.com>
No description provided.