-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
refactor(python): Split file of lazyframe methods #19937
base: main
Are you sure you want to change the base?
Conversation
40e6d00
to
0bca62c
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #19937 +/- ##
===========================================
+ Coverage 59.28% 79.48% +20.19%
===========================================
Files 1555 1556 +1
Lines 216180 216181 +1
Branches 2456 2456
===========================================
+ Hits 128155 171824 +43669
+ Misses 87467 43799 -43668
Partials 558 558 ☔ View full report in Codecov by Sentry. |
This on itself is fine, though we are not going to merge this because we want to revert all the splits. It is a requirement needed to get Polars working in WASM. See more context: PyO3/pyo3#2517 |
@ritchie46 Thanks for the reply, but do that restriction really still exist today? |
Indeed. My method works but requires using a custom LLVM & Rust compiler to support multiple pymethods with Wasm. I'm working on upstreaming the changes required to LLVM here. EDIT: The above PR is merged, but more work is still required. Another PR to LLVM will be needed, and then we must wait for the changes to hit |
@georgestagg Thanks, amazing! @ritchie46 Given that removing multiple pymethods would make the code significantly less readable, might it be better to keep the file split in favor of llvm/llvm-project#111008? |
@ritchie46 Could you reopen this? |
@georgestagg that's awesome. Could I contact you/ have a quick call? I'd like to learn more about this. |
0bca62c
to
61b09e2
Compare
@ritchie46 Sure, feel free to contact me at georgestagg [at] gmail [dot] com |
As with DataFrame, it would be better to split the io-related functions into separate files for LazyFrame as well.