Skip to content

migrate: convert missing-values.ipynb from pandas to polars - #11

Merged
brentomagic merged 3 commits into
mainfrom
missing-values-conversion-to-polars
Jul 28, 2026
Merged

migrate: convert missing-values.ipynb from pandas to polars#11
brentomagic merged 3 commits into
mainfrom
missing-values-conversion-to-polars

Conversation

@meshackamadi

Copy link
Copy Markdown
Collaborator

Summary

Converts the missing-values.ipynb chapter from pandas to polars, covering explicit and implicit missing value handling.

Changes

Code

  • .isna().is_null()
  • .notna().is_not_null()
  • .fillna().fill_null()
  • .dropna().drop_nulls()
  • method="ffill"strategy="forward"
  • method="bfill"strategy="backward"
  • .replace({-99: pd.NA}).with_columns(pl.col("*").replace(-99, None))

Text

  • Updated prerequisites to mention polars
  • Updated terminology: "NA" → "null"
  • Updated function names to polars equivalents

Testing

  • All code blocks execute successfully
  • Missing value handling produces expected results
  • Notebook renders without errors

Files Modified

  • missing-values.ipynb

@brentomagic brentomagic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the migration — the core API swaps (is_null / fill_null / drop_nulls / replace / pivot / Enum) look solid, and the empty-groups join pattern works. Before merge, please fix:

Blocking

  1. null vs NaN teaching is incorrect
    The numbers example still includes np.nan, and the prose says both kinds of missing value are found with .is_null(). In polars, NaN is not null:

    None  → null  → is_null() True
    NaN   → NaN   → is_null() False  (use is_nan())
    

    Either drop np.nan from the example and teach a single missing sentinel (null / None), or explicitly teach the difference with is_null() vs is_nan(). As written, this will mislead readers.

  2. Wrong notice after the “all null” filter
    After:

    null_exp_df.filter(~pl.all_horizontal(pl.all().is_null()))

    the text says: “Notice that the only the last all null column was removed.”
    That filter drops all-null rows, not columns — and column C (all null) remains. Update or remove that sentence. Also worth showing the idiomatic polars form if you want “drop rows where all values are null”: drop_nulls(how="all").

  3. limit= example doesn’t match the prose

    • Comment says “limit of 1”, code uses limit=2.
    • Surrounding text follows a per-column fill-value example, but the code switched to strategy="forward".
      Prefer keeping the pedagogical intent: per-column fills with a limit, e.g. with_columns([...].fill_null(value).forward_fill(limit=1)...) — or rewrite the prose to match forward-fill.

Medium

  1. Grammar: “an `null`” → “a `null`”.
  2. Leftover NA terminology: section header ### Adding NA values, and implicit-missing bullets still say `NA` while the rest of the chapter moved to null.
  3. Kernelspec display_name: ".venv" is local env noise — prefer a generic Python 3 kernel name.

Nit

  1. “As a convenience, there is also an .is_not_null() method” — wrap .is_not_null() in backticks for consistency.

@brentomagic

Copy link
Copy Markdown
Collaborator

Review notes

Thanks for the migration — the core API swaps (is_null / fill_null / drop_nulls / replace / pivot / Enum) look solid, and the empty-groups join pattern works. Before merge, please fix:

Blocking

  1. null vs NaN teaching is incorrect
    The numbers example still includes np.nan, and the prose says both kinds of missing value are found with .is_null(). In polars, NaN is not null:

    None  → null  → is_null() True
    NaN   → NaN   → is_null() False  (use is_nan())
    

    Either drop np.nan from the example and teach a single missing sentinel (null / None), or explicitly teach the difference with is_null() vs is_nan(). As written, this will mislead readers.

  2. Wrong notice after the “all null” filter
    After:

    null_exp_df.filter(~pl.all_horizontal(pl.all().is_null()))

    the text says: “Notice that the only the last all null column was removed.”
    That filter drops all-null rows, not columns — and column C (all null) remains. Update or remove that sentence. Also worth showing the idiomatic polars form if you want “drop rows where all values are null”: drop_nulls(how="all").

  3. limit= example doesn’t match the prose

    • Comment says “limit of 1”, code uses limit=2.
    • Surrounding text follows a per-column fill-value example, but the code switched to strategy="forward".
      Prefer keeping the pedagogical intent: per-column fills with a limit, or rewrite the prose to match forward-fill.

Medium

  1. Grammar: “an null” → “a null”.
  2. Leftover NA terminology: section header ### Adding NA values, and implicit-missing bullets still say NA while the rest of the chapter moved to null.
  3. Kernelspec display_name: ".venv" is local env noise — prefer a generic Python 3 kernel name.

Nit

  1. “As a convenience, there is also an .is_not_null() method” — wrap .is_not_null() in backticks for consistency.

@meshackamadi

Copy link
Copy Markdown
Collaborator Author

Blocking Issues Fixed

Fixed;

  • Null Vs Nan teaching
  • Accurate All-Null Filter & Prose
  • Matching limit Parameter
  • Kernelspec Metadata
  • Grammar correction
  • NA Terminology fixed
  • Method formatting

@meshackamadi
meshackamadi requested a review from brentomagic July 22, 2026 22:58

@brentomagic brentomagic left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Re-reviewed the follow-up commits — thanks for addressing the feedback.

Checklist vs prior review

  • null vs NaN — fixed; now teaches is_null() vs is_nan() on the numbers column
  • all-null filter notice — fixed; correctly describes row filtering and that column C remains
  • limit= mismatch — fixed (limit=1 in comment and code)
  • Grammar / NA terminology / .is_not_null() backticks / kernelspec — fixed

LGTM — approved.

@brentomagic
brentomagic merged commit 3567d05 into main Jul 28, 2026
2 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.

2 participants