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

No deprecation warning, while calling a function of an imported module #15081

Closed
2 tasks done
stfdxv opened this issue Mar 15, 2024 · 3 comments
Closed
2 tasks done

No deprecation warning, while calling a function of an imported module #15081

stfdxv opened this issue Mar 15, 2024 · 3 comments
Labels
A-exceptions Area: exception handling bug Something isn't working python Related to Python Polars

Comments

@stfdxv
Copy link

stfdxv commented Mar 15, 2024

Checks

  • I have checked that this issue has not already been reported.
  • I have confirmed this bug exists on the latest version of Polars.

Reproducible example

# module1.py
import polars as pl


def map_c(df):
    return df.with_columns(
        pl.col("c").map_dict({float("inf"): None}, default=pl.first()).alias("mapped_c")
    )


if __name__ == "__main__":
    df = pl.DataFrame({"a": [1.0], "b": [2.0], "c": [3.0]})
    print(map_c(df))
# module2.py
import module1
import polars as pl


df = pl.DataFrame({"a": [1.0], "b": [2.0], "c": [3.0]})
print(module1.map_c(df))

Log output

No response

Issue description

map_c() is a function, which uses a deprecated polars function map_dict().

  1. module1.py calls map_c(), defined locally. The deprecation warning is emitted - OK.
C:\Users\someone\PycharmProjects\experiments\polars_no_depr_warn\module1.py:7: DeprecationWarning: `map_dict` is deprecated. It has been renamed to `replace`. The default behavior has changed to keep any values not present in the mapping unchanged. Pass `default=None` to keep existing behavior.
  pl.col("c").map_dict({float("inf"): None}, default=pl.first()).alias("mapped_c")
shape: (1, 4)
┌─────┬─────┬─────┬──────────┐
│ a   ┆ b   ┆ c   ┆ mapped_c │
│ --- ┆ --- ┆ --- ┆ ---      │
│ f64 ┆ f64 ┆ f64 ┆ f64      │
╞═════╪═════╪═════╪══════════╡
│ 1.0 ┆ 2.0 ┆ 3.0 ┆ 1.0      │
└─────┴─────┴─────┴──────────┘
  1. module2.py calls the same function map_c(), which is imported from module1.py. No deprecation warning - not OK.
shape: (1, 4)
┌─────┬─────┬─────┬──────────┐
│ a   ┆ b   ┆ c   ┆ mapped_c │
│ --- ┆ --- ┆ --- ┆ ---      │
│ f64 ┆ f64 ┆ f64 ┆ f64      │
╞═════╪═════╪═════╪══════════╡
│ 1.0 ┆ 2.0 ┆ 3.0 ┆ 1.0      │
└─────┴─────┴─────┴──────────┘

P.S. Note the wrong value in mapped_c column. This is probably due to a bug, which I described in #15079. The absence of the deprecation warning makes it difficult to notice the degradation after polars upgrade.

Expected behavior

Deprecation warnings should be emitted no matter the function was defined locally or imported from another module.

Installed versions

--------Version info---------
Polars:               0.20.16-rc.1
Index type:           UInt32
Platform:             Windows-10-10.0.19041-SP0
Python:               3.10.5 (tags/v3.10.5:f377153, Jun  6 2022, 16:14:13) [MSC v.1929 64 bit (AMD64)]

----Optional dependencies----
adbc_driver_manager:  <not installed>
cloudpickle:          <not installed>
connectorx:           <not installed>
deltalake:            <not installed>
fastexcel:            <not installed>
fsspec:               <not installed>
gevent:               <not installed>
hvplot:               <not installed>
matplotlib:           <not installed>
numpy:                1.24.3
openpyxl:             <not installed>
pandas:               2.1.1
pyarrow:              13.0.0
pydantic:             <not installed>
pyiceberg:            <not installed>
pyxlsb:               <not installed>
sqlalchemy:           <not installed>
xlsx2csv:             <not installed>
xlsxwriter:           <not installed>
@stfdxv stfdxv added bug Something isn't working needs triage Awaiting prioritization by a maintainer python Related to Python Polars labels Mar 15, 2024
@ritchie46
Copy link
Member

I am not sure. Users of a library that is written in Polars shouldn't see that deprecation warning. That warning is intended for the developers of that library, not the users up the stack.

@MarcoGorelli
Copy link
Collaborator

Sure but like this direct users of Polars who have defined their own convenience functions (e.g. @stfdxv ) risk not seeing the warning

The other extreme is to do what pandas currently does which is to always raise FutureWarning, which is always shown. This is too noisy in my opinion, users see warnings which they can't do anything about.

A middle-ground option, which I'm hoping pandas will move to too (long discussion here pandas-dev/pandas#54970), is:

  • start by raising DeprecationWarning. This will be seen my library developers and end users directly invoking deprecated functions
  • after some time, change it to FutureWarning. This will be visible by everyone, including end users who have defined their own convenience functions (e.g. @stfdxv )
  • finally, enforce the deprecation

Example following the Polars policy:

  • Polars 0.20.11 deprecates pl.col('a').meta.write_json in favour of pl.col('a').meta.serialize, and emits a DeprecationWarning
  • In Polars 1.1.0 (some time in the middle of the 1.x series? When there's a feeling that the next minor release will be the final non-breaking one? Up for discussion), the DeprecationWarning changes to FutureWarning
  • In Polars 2.0.0, Expr.meta.write_json is removed

@stinodego
Copy link
Member

stinodego commented Mar 15, 2024

Thanks for the report, but this is not an issue with Polars.

The Python docs are really clear about this. DeprecationWarnings are ignored outside the current main.

Also, FutureWarning is not applicable here. It is intended for end users, not for developers. Polars should not throw any FutureWarnings.

If you want to see DeprecationWarnings outside of the main module, you can explicitly opt in to that:

import warnings

warnings.filterwarnings("always", category=DeprecationWarning)

@stinodego stinodego closed this as not planned Won't fix, can't repro, duplicate, stale Mar 15, 2024
@stinodego stinodego removed the needs triage Awaiting prioritization by a maintainer label Mar 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-exceptions Area: exception handling bug Something isn't working python Related to Python Polars
Projects
None yet
Development

No branches or pull requests

4 participants