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

Replicate py-polars API surface for streaming IPC formats #249

Merged
merged 3 commits into from
Aug 16, 2024

Conversation

H-Plus-Time
Copy link
Contributor

@H-Plus-Time H-Plus-Time commented Jul 25, 2024

TLDR: Solves #109

More or less the IPC Stream methods are straight copies of the IPC File (Feather) ones, swapping out the IpcReader, IpcWriter for their streaming equivalents; the API should be identical to py-polars (with the exception of file-like objects as input for read_ipc, read_ipc_stream - not much point adding that until streaming IO is exposed upstream).

I've left the docstrings basically untouched, let me know if you want those tweaked (the @param s appear to have drifted over time).

…ts to cover it, clarify distinction between IPC File and Stream in docstrings
@Bidek56
Copy link
Collaborator

Bidek56 commented Jul 25, 2024

@H-Plus-Time Thanks for your contribution, please ensure the docstrings match the function params. Thx

@Bidek56
Copy link
Collaborator

Bidek56 commented Jul 25, 2024

This code: pl.read_ipc('https://paste.c-net.org/ViperMoronic') still does not work in py-polars why would it work in nodejs-polars?

@H-Plus-Time
Copy link
Contributor Author

pl.read_ipc_stream is what's required there - the read_ipc/readIPC methods in both py-polars and nodejs-polars assume the target is in the IPC File format (the resource at that url is in the stream format, which lacks the footer read_ipc is expecting).

@Bidek56
Copy link
Collaborator

Bidek56 commented Jul 27, 2024

@H-Plus-Time please run: yarn run lint:ts:fix on your PR and push the changes. Thx

@universalmind303 universalmind303 merged commit b243288 into pola-rs:main Aug 16, 2024
9 checks passed
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.

None yet

3 participants