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

improve unpack_list error #607

Merged
merged 5 commits into from
Dec 23, 2023
Merged

improve unpack_list error #607

merged 5 commits into from
Dec 23, 2023

Conversation

sorhawell
Copy link
Collaborator

@sorhawell sorhawell commented Dec 18, 2023

resolves #600

this PR improves the bad error message for trailing commas for internal function unpack_list().

Also the function now has .context= and .call args just as unwrap().

I have provided context for any use of unpack_list()

In future when/if extendr+polars supports/allows trailing commas. This extra error styling can be removed again.

examples

> unpack_list(1,2,)
Error: Execution halted with the following contexts
0: During function call [unpack_list(1, 2, )]
1: trailing argument commas are not (yet) supported with polars
2: argument is missing, with no default
> pl$DataFrame()$select(1,2,)
Error: Execution halted with the following contexts
0: In R: in $select()
0: During function call [pl$DataFrame()$select(1, 2, )]
1: trailing argument commas are not (yet) supported with polars
2: argument is missing, with no default

@sorhawell sorhawell marked this pull request as ready for review December 18, 2023 16:41
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.

LGTM, can you add some tests and bump news?

Copy link
Collaborator

@eitsupi eitsupi left a comment

Choose a reason for hiding this comment

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

Thanks for working on this.

Could you also update unpack_bool_expr and related functions?

tests/testthat/test-dotdotdot.R Show resolved Hide resolved
tests/testthat/test-dotdotdot.R Show resolved Hide resolved
tests/testthat/test-dotdotdot.R Show resolved Hide resolved
@sorhawell
Copy link
Collaborator Author

> pl$DataFrame(list(list(1,2,3)))
shape: (3, 1)
┌────────────┐
│ new_column │
│ ---        │
│ list[f64]  │
╞════════════╡
│ [1.0]      │
│ [2.0]      │
│ [3.0]      │
└────────────┘
> pl$DataFrame(list(list(1,2,3)),"a")
shape: (1, 2)
┌───────────────────────┬──────────────┐
│ new_columnnew_column_1 │
│ ------          │
│ list[list[f64]]       ┆ str          │
╞═══════════════════════╪══════════════╡
│ [[1.0], [2.0], [3.0]] ┆ a            │
└───────────────────────┴──────────────┘

@etiennebacher
Copy link
Collaborator

Apart from moving the functions dealing with ... in a separate dotdotdot.R, is there anything else to do here?

@eitsupi eitsupi merged commit 7590e75 into main Dec 23, 2023
17 checks passed
@eitsupi eitsupi deleted the unpack_list_errors branch December 23, 2023 00:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error messages from with_columns() is difficult to understand when extra comma
3 participants