Repository navigation
fix: data_editor corrupting int8/uint8/float16 columns on edit - #10211
Conversation
_convert_value hand-enumerated only the 16/32/64-bit integer and float widths, so editing a cell in an Int8, UInt8, Int128, UInt128 or Float16 column fell through to str(value): pandas coerced the whole column to object, and polars raised "Data editor does not support this type of data". Use narwhals' dtype-category predicates (is_integer/is_float) instead of enumerating widths by hand, which covers every width. The category methods are used rather than adding nw.Int8/nw.Float16 explicitly because nw.Float16 does not exist on the minimum supported narwhals. Fixes marimo-team#10210
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
All contributors have signed the CLA ✍️ ✅ |
|
I have read the CLA Document and I hereby sign the CLA |
| def test_convert_value_with_dtype_int8(self): | ||
| """Test Int8 conversion with dtype.""" | ||
| result = _convert_value("42", None, nw.Int8) | ||
| assert result == 42 | ||
| assert isinstance(result, int) | ||
|
|
||
| def test_convert_value_with_dtype_uint8(self): | ||
| """Test UInt8 conversion with dtype.""" | ||
| result = _convert_value("42", None, nw.UInt8) | ||
| assert result == 42 | ||
| assert isinstance(result, int) | ||
|
|
||
| def test_convert_value_with_dtype_int128(self): | ||
| """Test Int128 conversion with dtype.""" | ||
| result = _convert_value("42", None, nw.Int128) | ||
| assert result == 42 | ||
| assert isinstance(result, int) | ||
|
|
||
| def test_convert_value_with_dtype_uint128(self): | ||
| """Test UInt128 conversion with dtype.""" | ||
| result = _convert_value("42", None, nw.UInt128) | ||
| assert result == 42 | ||
| assert isinstance(result, int) | ||
|
|
||
| def test_convert_value_with_dtype_float16(self): | ||
| """Test Float16 conversion with dtype.""" | ||
| float16 = getattr(nw, "Float16", None) | ||
| if float16 is None: |
There was a problem hiding this comment.
could we instead parametrize these tests. it's verbose
| elif hasattr(dtype, "is_float") and dtype.is_float(): | ||
| # Any float width (Float32/Float64, plus Float16 where the | ||
| # installed narwhals exposes it). | ||
| return float(value) | ||
| elif ( | ||
| dtype == nw.Int16 | ||
| or dtype == nw.Int32 | ||
| or dtype == nw.Int64 | ||
| or dtype == nw.UInt16 | ||
| or dtype == nw.UInt32 | ||
| or dtype == nw.UInt64 | ||
| ): | ||
| elif hasattr(dtype, "is_integer") and dtype.is_integer(): | ||
| # Any signed/unsigned integer width. Enumerating them by hand | ||
| # missed the 8- and 128-bit widths, so edits to Int8/UInt8/ | ||
| # Int128/UInt128 columns fell through to str() below and | ||
| # silently coerced the whole column to object/string. |
There was a problem hiding this comment.
I would reduce the comments, it's unecessary.
|
Thanks @Light2Dark! Addressed both:
FWIW the two red checks look unrelated to this change — a snapshot diff in |
|
🚀 Development release published. You may be able to view the changes at https://marimo.app?v=0.23.15-dev34 |
… (#10304) **This pull request was authored by a coding agent.** ## What / why Editing a cell in a **pandas `float16`** column silently corrupts the whole column: the edited value is stored as a string and the column dtype flips to `object`. ```python import pandas as pd from marimo._plugins.ui._impl.data_editor import apply_edits df = pd.DataFrame({"a": pd.array([1.0, 2.0, 3.0], dtype="float16")}) apply_edits(df, {"edits": [{"rowIdx": 0, "columnId": "a", "value": "5.5"}]}) # before: dtype object, values ['5.5', 2.0, 3.0] (corrupted) # after: dtype float16-compatible numeric, values [5.5, 2.0, 3.0] ``` #10211 addressed `int8`/`uint8`/`float32` by switching to `dtype.is_integer()` / `dtype.is_float()`, but `float16` still degrades: narwhals (2.x) has no `Float16`, so it reports pandas `float16` as `nw.Unknown`. Both predicate checks return `False` and `_convert_value` falls through to `return str(value)`. The dtype-preservation test the PR added for `float16` is currently **red on `main`**: ``` FAILED tests/_plugins/ui/_impl/test_data_editor.py::test_apply_edits_dataframe_small_width_numeric_preserves_dtype[float16] ``` ## Fix For an unsupported/`Unknown` dtype, fall through to the existing original-value coercion (`_convert_value` already handles `int`/`float`/`str`/…) instead of stringifying. For a numeric column the original value is a Python `float`/`int`, so the edit stays numeric. This is a superset fix — any dtype narwhals can't classify but whose values are numeric now round-trips correctly. ## Tests - The previously-failing `test_apply_edits_dataframe_small_width_numeric_preserves_dtype[float16]` now passes. - Added `test_convert_value_unknown_dtype_coerces_from_original`, which pins the fall-through directly against `nw.Unknown` (independent of narwhals' evolving float16 mapping). - `pytest tests/_plugins/ui/_impl/test_data_editor.py` → 107 passed; `ruff check`/`ruff format` clean. Follow-up to #10211.
…o-team#10211) ### What / why Editing a cell in a numeric column whose dtype is `Int8`, `UInt8`, `Int128`, `UInt128`, or `Float16` silently corrupted the column. `_convert_value` hand-enumerated only the 16/32/64-bit integer and float widths, so any other width fell through to `return str(value)`: - **pandas** — the edited value became a string and the whole column was coerced to `object`. - **polars** — the edit failed with a misleading `ValueError: Data editor does not support this type of data`. ```python import pandas as pd from marimo._plugins.ui._impl.data_editor import apply_edits df = pd.DataFrame({"a": pd.array([1, 2, 3], dtype="int8")}) res = apply_edits(df, {"edits": [{"rowIdx": 0, "columnId": "a", "value": "5"}]}) print(res["a"].dtype, list(res["a"])) # before: object ['5', 2, 3] (corrupted) # after: int64 [5, 2, 3] ``` ### Fix Replace the hand-enumerated width lists with narwhals' dtype-category predicates (`dtype.is_integer()` / `dtype.is_float()`), which cover every integer/float width and can't miss one again. The category methods are used rather than adding `nw.Int8`/`nw.Float16` explicitly because `nw.Float16` does not exist as an attribute on the minimum supported narwhals (2.0.0), so referencing it would `AttributeError`. ### Tests Added direct `_convert_value` tests (Int8/UInt8/Int128/UInt128/Float16) and dataframe-level dtype-preservation tests (pandas int8/uint8/float16 + polars Int8). All fail on the previous code and pass now. `pytest tests/_plugins/ui/_impl/test_data_editor.py` → 105 passed; `ruff check`/`ruff format` clean. Fixes marimo-team#10210
…o-team#10211) (marimo-team#10304) **This pull request was authored by a coding agent.** ## What / why Editing a cell in a **pandas `float16`** column silently corrupts the whole column: the edited value is stored as a string and the column dtype flips to `object`. ```python import pandas as pd from marimo._plugins.ui._impl.data_editor import apply_edits df = pd.DataFrame({"a": pd.array([1.0, 2.0, 3.0], dtype="float16")}) apply_edits(df, {"edits": [{"rowIdx": 0, "columnId": "a", "value": "5.5"}]}) # before: dtype object, values ['5.5', 2.0, 3.0] (corrupted) # after: dtype float16-compatible numeric, values [5.5, 2.0, 3.0] ``` marimo-team#10211 addressed `int8`/`uint8`/`float32` by switching to `dtype.is_integer()` / `dtype.is_float()`, but `float16` still degrades: narwhals (2.x) has no `Float16`, so it reports pandas `float16` as `nw.Unknown`. Both predicate checks return `False` and `_convert_value` falls through to `return str(value)`. The dtype-preservation test the PR added for `float16` is currently **red on `main`**: ``` FAILED tests/_plugins/ui/_impl/test_data_editor.py::test_apply_edits_dataframe_small_width_numeric_preserves_dtype[float16] ``` ## Fix For an unsupported/`Unknown` dtype, fall through to the existing original-value coercion (`_convert_value` already handles `int`/`float`/`str`/…) instead of stringifying. For a numeric column the original value is a Python `float`/`int`, so the edit stays numeric. This is a superset fix — any dtype narwhals can't classify but whose values are numeric now round-trips correctly. ## Tests - The previously-failing `test_apply_edits_dataframe_small_width_numeric_preserves_dtype[float16]` now passes. - Added `test_convert_value_unknown_dtype_coerces_from_original`, which pins the fall-through directly against `nw.Unknown` (independent of narwhals' evolving float16 mapping). - `pytest tests/_plugins/ui/_impl/test_data_editor.py` → 107 passed; `ruff check`/`ruff format` clean. Follow-up to marimo-team#10211.
What / why
Editing a cell in a numeric column whose dtype is
Int8,UInt8,Int128,UInt128, orFloat16silently corrupted the column._convert_valuehand-enumerated only the 16/32/64-bit integer and float widths, so any other width fell through toreturn str(value):object.ValueError: Data editor does not support this type of data.Fix
Replace the hand-enumerated width lists with narwhals' dtype-category predicates (
dtype.is_integer()/dtype.is_float()), which cover every integer/float width and can't miss one again. The category methods are used rather than addingnw.Int8/nw.Float16explicitly becausenw.Float16does not exist as an attribute on the minimum supported narwhals (2.0.0), so referencing it wouldAttributeError.Tests
Added direct
_convert_valuetests (Int8/UInt8/Int128/UInt128/Float16) and dataframe-level dtype-preservation tests (pandas int8/uint8/float16 + polars Int8). All fail on the previous code and pass now.pytest tests/_plugins/ui/_impl/test_data_editor.py→ 105 passed;ruff check/ruff formatclean.Fixes #10210