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

feat: rewrite arrow_to_rdf and arrow_to_rseries_result #727

Merged
merged 6 commits into from
Jan 21, 2024

Conversation

eitsupi
Copy link
Collaborator

@eitsupi eitsupi commented Jan 21, 2024

Fix #725, part of #677

After rewriting the contents of the functions with reference to Python Polars, the bug #725 seems to have been fixed.
(I'm not sure where the bug was occurring after all)

Other improvements:

  • as_polars_df for RecordBatch now works.
  • as_polars_series for Array and ChunkedArray are added.

These changes have completely ported pl$from_arrow's functionality to as_polars_*, so we can deprecate pl$from_arrow in a follow-up PR.

R/construction.R Outdated Show resolved Hide resolved
R/construction.R Show resolved Hide resolved
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.

Is there also a way to add a test reproducing #725 or would that be too hard to add in unit test?

@eitsupi
Copy link
Collaborator Author

eitsupi commented Jan 21, 2024

Is there also a way to add a test reproducing #725 or would that be too hard to add in unit test?

Of course I'll add it. Something like below (this crashes in current release version)

  da_string = arrow::Array$create(
    factor(c("x", "y", "z"))
  )

  da_large_string = da_string$cast(
    arrow::dictionary(
      index_type = arrow::uint32(),
      value_type = arrow::large_utf8()
    )
  )

  at = arrow::arrow_table(foo = da_string, bar = da_large_string)

as_polars_df.ArrowTabular(at)

@eitsupi eitsupi marked this pull request as ready for review January 21, 2024 14:24
@eitsupi eitsupi changed the title fix: rewrite arrow_to_rdf and arrow_to_rseries_result feat: rewrite arrow_to_rdf and arrow_to_rseries_result Jan 21, 2024
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.

Thanks!

@eitsupi eitsupi merged commit 244892f into main Jan 21, 2024
17 checks passed
@eitsupi eitsupi deleted the from-arrow-table branch January 21, 2024 14:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants