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, rust): Validate schema in melt #15514

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

edavisau
Copy link
Contributor

@edavisau edavisau commented Apr 7, 2024

Fixes #13493

This assumes that

  1. There are no duplicate column names among id_vars and value_vars
  2. All column names in id_vars and value_vars exist in the schema

Condition 1 prevents invalid dataframes from being constructed as follows

import polars as pl

pl.DataFrame([pl.Series("a", [1,2,3])]).melt(id_vars=["a", "a", "a", "a"], value_vars=["a", "a"])

# shape: (6, 6)
# ┌─────┬─────┬─────┬─────┬──────────┬───────┐
# │ a   ┆ a   ┆ a   ┆ a   ┆ variable ┆ value │
# │ --- ┆ --- ┆ --- ┆ --- ┆ ---      ┆ ---   │
# │ i64 ┆ i64 ┆ i64 ┆ i64 ┆ str      ┆ i64   │
# ╞═════╪═════╪═════╪═════╪══════════╪═══════╡
# │ 1   ┆ 1   ┆ 1   ┆ 1   ┆ a        ┆ 1     │
# │ 2   ┆ 2   ┆ 2   ┆ 2   ┆ a        ┆ 2     │
# │ 3   ┆ 3   ┆ 3   ┆ 3   ┆ a        ┆ 3     │
# │ 1   ┆ 1   ┆ 1   ┆ 1   ┆ a        ┆ 1     │
# │ 2   ┆ 2   ┆ 2   ┆ 2   ┆ a        ┆ 2     │
# │ 3   ┆ 3   ┆ 3   ┆ 3   ┆ a        ┆ 3     │
# └─────┴─────┴─────┴─────┴──────────┴───────┘

@github-actions github-actions bot added fix Bug fix python Related to Python Polars rust Related to Rust Polars labels Apr 7, 2024
Copy link

codecov bot commented Apr 7, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.26%. Comparing base (92902e6) to head (6c86d88).

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #15514   +/-   ##
=======================================
  Coverage   81.25%   81.26%           
=======================================
  Files        1371     1371           
  Lines      175772   175785   +13     
  Branches     2531     2531           
=======================================
+ Hits       142831   142848   +17     
+ Misses      32464    32462    -2     
+ Partials      477      475    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@edavisau edavisau marked this pull request as draft April 13, 2024 01:56
Copy link

codspeed-hq bot commented Apr 13, 2024

CodSpeed Performance Report

Merging #15514 will not alter performance

Comparing edavisau:melt-panic (6c86d88) with main (92902e6)

Summary

✅ 21 untouched benchmarks

@edavisau edavisau marked this pull request as ready for review April 13, 2024 02:28
@edavisau
Copy link
Contributor Author

Not sure why the unit tests failed in the previous commit, passing now. Though CodSpeed is now failing 😮

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 rust Related to Rust Polars
Projects
None yet
Development

Successfully merging this pull request may close these issues.

melt panics when id_vars is a non-existing column
1 participant