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

fix(python): Issue correct PolarsInefficientMapWarning for lshift/rshift operations #12385

Merged

Conversation

MarcoGorelli
Copy link
Collaborator

closes #12384

@MarcoGorelli MarcoGorelli marked this pull request as ready for review November 11, 2023 11:08
@stinodego stinodego changed the title bug(python): dont warn about inefficient map_elements replacement for lshift/rshift fix(python): Do not warn about inefficient map_elements replacement for lshift/rshift operators Nov 12, 2023
@github-actions github-actions bot added fix Bug fix python Related to Python Polars labels Nov 12, 2023
@stinodego stinodego changed the title fix(python): Do not warn about inefficient map_elements replacement for lshift/rshift operators fix(python): Do not issue InefficientMapWarning for lshift/rshift operators Nov 12, 2023
@stinodego stinodego changed the title fix(python): Do not issue InefficientMapWarning for lshift/rshift operators fix(python): Do not issue PolarsInefficientMapWarning for lshift/rshift operators Nov 12, 2023
stinodego
stinodego previously approved these changes Nov 12, 2023
@stinodego stinodego changed the title fix(python): Do not issue PolarsInefficientMapWarning for lshift/rshift operators fix(python): Do not issue PolarsInefficientMapWarning for lshift/rshift operations Nov 12, 2023
@cmdlineluser
Copy link
Contributor

Perhaps #12090 (comment) could be suggested as an alternative.

As a workaround for now you can replace a << b with a * 2**b and a >> b with a.floordiv(2**b).

@stinodego stinodego dismissed their stale review November 12, 2023 07:10

Consider alternative proposed by cmdlineuser

@MarcoGorelli
Copy link
Collaborator Author

nice! I'll give this a go

@MarcoGorelli MarcoGorelli changed the title fix(python): Do not issue PolarsInefficientMapWarning for lshift/rshift operations fix(python): Issue correct PolarsInefficientMapWarning for lshift/rshift operations Nov 12, 2023
@MarcoGorelli
Copy link
Collaborator Author

That seems to work wonderfully, thanks!

Demo with the originally reported issue:

In [3]: 
   ...: df = pl.DataFrame({'ipvers': [4, 4], 'prefix': ['172.16.2.0/24', '172.16.2.101/32'], 'prefixlen': [24, 32]})
   ...: (
   ...:     df
   ...:     .filter(pl.col('ipvers') == 4)
   ...:     .with_columns(
   ...:         netmask = pl.col('prefixlen')
   ...:         .map_elements(lambda x: (0xffffffff >> (32 - x)) &
   ...:                       0xffffffff)
   ...:     )
   ...: )
<ipython-input-3-cf7f706f16ef>:7: PolarsInefficientMapWarning: 
Expr.map_elements is significantly slower than the native expressions API.
Only use if you absolutely CANNOT implement your logic otherwise.
In this case, you can replace your `map_elements` with the following:
  - pl.col("prefixlen").map_elements(lambda x: ...)
  + (4294967295 / (2**(32 - pl.col("prefixlen")))).cast(pl.Int64) & 4294967295

  .map_elements(lambda x: (0xffffffff >> (32 - x)) &
Out[3]: 
shape: (2, 4)
┌────────┬─────────────────┬───────────┬────────────┐
│ ipversprefixprefixlennetmask    │
│ ------------        │
│ i64stri64i64        │
╞════════╪═════════════════╪═══════════╪════════════╡
│ 4172.16.2.0/242416777215   │
│ 4172.16.2.101/32324294967295 │
└────────┴─────────────────┴───────────┴────────────┘

In [4]: 
   ...: df = pl.DataFrame({'ipvers': [4, 4], 'prefix': ['172.16.2.0/24', '172.16.2.101/32'], 'prefixlen': [24, 32]})
   ...: (
   ...:     df
   ...:     .filter(pl.col('ipvers') == 4)
   ...:     .with_columns(
   ...:         netmask = (4294967295 / (2**(32 - pl.col("prefixlen")))).cast(pl.Int64) & 4294967295
   ...:     )
   ...: )
Out[4]: 
shape: (2, 4)
┌────────┬─────────────────┬───────────┬────────────┐
│ ipversprefixprefixlennetmask    │
│ ------------        │
│ i64stri64i64        │
╞════════╪═════════════════╪═══════════╪════════════╡
│ 4172.16.2.0/242416777215   │
│ 4172.16.2.101/32324294967295 │
└────────┴─────────────────┴───────────┴────────────┘

Copy link
Contributor

@stinodego stinodego left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, thanks Marco!

@ritchie46 ritchie46 merged commit a380439 into pola-rs:main Nov 13, 2023
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fix Bug fix python Related to Python Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Incorrect "inefficient map_elements" warning on Python3.11
4 participants