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.

Sign up for GitHub

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

Jump to bottom

Rework Expr operators and errors #346

Merged
merged 14 commits into from
Aug 7, 2023
Merged

Rework Expr operators and errors #346

merged 14 commits into from
Aug 7, 2023

Conversation

sorhawell
Copy link
Collaborator

to solve #344

@etiennebacher oups I also tried to solve that issue :)

I think we can combine #346 and #345

@etiennebacher
Copy link
Collaborator

I'd say let's take your fix and add my tests here? ;)

@etiennebacher etiennebacher mentioned this pull request Aug 3, 2023
Closed
@sorhawell
Copy link
Collaborator Author

derived wrap_e from robj_to! + update all binary operator implementation to use new RPolarsErr handling

etiennebacher
etiennebacher requested changes Aug 4, 2023
View reviewed changes
Copy link
Collaborator

@etiennebacher etiennebacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as before, I can't review the Rust part. Minor things to change

R/expr__expr.R Outdated Show resolved Hide resolved
R/expr__expr.R Outdated Show resolved Hide resolved
tests/testthat/test-expr.R Outdated Show resolved Hide resolved
etiennebacher
etiennebacher requested changes Aug 4, 2023
View reviewed changes
NEWS.md Outdated Show resolved Hide resolved
R/expr__expr.R Outdated Show resolved Hide resolved
@etiennebacher etiennebacher merged commit a123dd9 into main Aug 7, 2023
11 checks passed
@etiennebacher etiennebacher deleted the fix_binary_op_literals branch August 7, 2023 11:44
@etiennebacher etiennebacher mentioned this pull request Aug 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@etiennebacher etiennebacher etiennebacher approved these changes

@eitsupi eitsupi Awaiting requested review from eitsupi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sorhawell @etiennebacher