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

FileAttachment.arquero #1509

Merged
merged 10 commits into from
Jul 21, 2024
Merged

FileAttachment.arquero #1509

merged 10 commits into from
Jul 21, 2024

Conversation

mbostock
Copy link
Member

@mbostock mbostock commented Jul 9, 2024

@mbostock mbostock requested a review from Fil July 9, 2024 20:52
@Fil
Copy link
Contributor

Fil commented Jul 10, 2024

A question would be, should the .arquero method also work on a JSON or CSV file (or even a "fixed" file, for aq.loadFixed)?

@mbostock
Copy link
Member Author

Okay, I added support for CSV, TSV, and JSON.

@Fil
Copy link
Contributor

Fil commented Jul 11, 2024

I've added a bit of documentation.

I this this raises another question: if we want to generalize this approach, shouldn't we have a duckdb method as well, e.g. FileAttachment("data.csv").duckdb()?

@mbostock
Copy link
Member Author

We certainly could add a DuckDB method, but I don’t think that should block this PR, and I think it makes slightly less sense in the DuckDB case because typically a database has multiple tables and so you don’t want to give the file attachment primacy (i.e., it’s better to use DuckDBClient.of to accommodate multiple tables than FileAttachment.duckdb which is limited to a single table).

@mbostock mbostock marked this pull request as ready for review July 20, 2024 17:50
@mbostock
Copy link
Member Author

Added support for Apache Parquet files (and preloading which is a bit tricky because we only need to additionally preload parquet-wasm when using an Apache Parquet file with Arquero). Made the Arquero loader detection a bit more robust considering the file extension when the MIME type isn’t sufficient. Also tweaked the documentation a bit.

PTAL @Fil thank you!

@mbostock mbostock enabled auto-merge (squash) July 20, 2024 17:52
@mbostock mbostock merged commit 7bfdab0 into main Jul 21, 2024
4 checks passed
@mbostock mbostock deleted the mbostock/file-arquero branch July 21, 2024 15:26
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.

2 participants