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

polars -> narwhals #714

Merged
merged 11 commits into from
Nov 22, 2024
Merged

polars -> narwhals #714

merged 11 commits into from
Nov 22, 2024

Conversation

s3alfisc
Copy link
Member

Full migration from all code parts that rely on polars to narwhals. Drops polars dependency.

@s3alfisc s3alfisc linked an issue Nov 13, 2024 that may be closed by this pull request
@s3alfisc
Copy link
Member Author

s3alfisc commented Nov 19, 2024

Hi @MarcoGorelli ,I am trying to fully deprecate the polars dependency in favor of narwhals. I am using pl.count() for which narwhals does not seem to support a direct equivalent. Is that correct? If yes, is there any other way to rewrite

    agg_expressions.append(pl.count(depvars[0]).alias("count"))

to narwhals supported syntax?

@s3alfisc
Copy link
Member Author

Actually, this isn't even true, of course there is a count in narwhals: link.

@s3alfisc
Copy link
Member Author

This should work now =)

Copy link

codecov bot commented Nov 21, 2024

Codecov Report

Attention: Patch coverage is 71.42857% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
pyfixest/estimation/feols_compressed_.py 72.97% 10 Missing ⚠️
pyfixest/estimation/feols_.py 60.00% 2 Missing ⚠️
Flag Coverage Δ
core-tests 77.32% <71.42%> (-0.59%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
pyfixest/estimation/feols_.py 83.20% <60.00%> (-0.08%) ⬇️
pyfixest/estimation/feols_compressed_.py 69.23% <72.97%> (-14.21%) ⬇️

🚨 Try these New Features:

@s3alfisc
Copy link
Member Author

s3alfisc commented Nov 21, 2024

Unit tests in tests/test_feols_compressed.py are now very slow - looks like I lose a lot of performance, likely by doing something that is not recommended 🤔

@s3alfisc
Copy link
Member Author

I think I completely misunderstood how narwhals works - after all, I now realized that narwhals does not copy code from pandas to polars and runs polars under the hood (not sure where I got this from, maybe because all the R translators of dplyr do this?), but simply translates code! So the performance drop came from running all computations via pandas instead of polars 🤦

@s3alfisc s3alfisc merged commit c08d447 into master Nov 22, 2024
8 of 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.

FeolsCompressed: Port code from polars to narwhals
1 participant