feat: Add {nw,DataFrame}.from_dicts

[felixgwilliams]

What type of PR is this? (check all applicable)

• 💾 Refactor
• ✨ Feature
• 🐛 Bug Fix
• 🔧 Optimization
• 📝 Documentation
• ✅ Test
• 🐳 Other

Related issues

• Related issue [Enh]: Add nw.from_dicts to convert a sequence of dictionaries representing rows to a data frame #3145
• Closes [Enh]: Add nw.from_dicts to convert a sequence of dictionaries representing rows to a data frame #3145

Checklist

• Code follows style guide (ruff)
• Tests added
• Documented the changes

If you have comments or can explain your changes, please do so below

This PR adds a from_dicts function and methods, that can be used to create a data frame from a sequence of dicts that represent rows. It is available for eager backends.

Tracking

• fix(python): Widen from_dicts to Iterable[Mapping[str, Any]] pola-rs/polars#24584

[FBruzzesi]

Thanks @felixgwilliams this looks promising and already close to the finish line.

I left a few comments, arguably nitpicks.

A couple of things that are missing:

• Exposing the functionalities in stable.v1
• A test to check for both empty data and no schema resulting in a shape=(0,0) dataframe.

The same as:

def test_from_dict_empty(eager_backend: EagerAllowed) -> None:
    result = nw.DataFrame.from_dict({}, backend=eager_backend)
    assert result.shape == (0, 0)

[FBruzzesi]

Thanks for all the adjustments @felixgwilliams - I would stall for a moment waiting for an opinion on #3148 (comment), aside that the feature seems ready 👌🏼

[felixgwilliams]

I noticed that when the schemas of the dicts is not consistent and schema is not specified, the behavior is different when using the PyArrow backend, because PyArrow only looks at the first row to establish the (see here), whereas pandas uses all the rows and polars seems to use the first 100 by default.

Do you think it's worth highlighting the differences in the docstring or do you think "If not specified, the schema will be inferred by the native library." covers it? I think it's surprising enough to be worth mentioning, but I'm conscious of not wanting to be too verbose.

[dangotbanned]

I noticed that when the schemas of the dicts is not consistent and schema is not specified, the behavior is different

• pyarrow only looks at the first row to establish the schema (see here)
• pandas uses all the rows
• polars uses the first 100 by default.

Well spotted!

Do you think it's worth highlighting the differences in the docstring or do you think

"If not specified, the schema will be inferred by the native library."

covers it?
I think it's surprising enough to be worth mentioning, but I'm conscious of not wanting to be too verbose.

Agreed, this does seem like it would be helpful to document, considering:

• the JSON context ([Enh]: Add nw.from_dicts to convert a sequence of dictionaries representing rows to a data frame #3145 (comment)) I framed this in
• missing keys are common enough to become a typing feature

Someone will get burned by this eventually 😳

What to do?

Docs

Although we could specify these differences in the docstring of from_dicts, I suspect we might also see the same kind of differences in:

• DataFrame.from_dict
• Series.from_iterable
• ... maybe others in API: io functions for v2 #2116

If that's the case, then we could benefit from some narrative docs in the user guide.
That way we could do side-by-side comparisons without clogging up the docstrings 😅

Note

Might be best to split that out into a follow-up issue?

[dangotbanned]

Tests

I noticed that when the schemas of the dicts is not consistent and schema is not specified, the behavior is different

Add tests for this then 😄

Each should have data with a schema change in row:

1. 2 (xfail pyarrow)
2. 99 (xfail pyarrow)
3. 101 (xfail pyarrow, polars)

I'd recommend using this fixture instead of eager_backend, but in this case it's because the input data size is larger than normal

narwhals/tests/conftest.py

Lines 320 to 323 in 63c5022

	@pytest.fixture(params=[el for el in TEST_EAGER_BACKENDS if not isinstance(el, str)])
	def eager_implementation(request: pytest.FixtureRequest) -> EagerAllowed:
	"""Use if a test is heavily parametric, skips `str` backend."""
	return request.param # type: ignore[no-any-return]

[felixgwilliams]

Add tests for this then 😄

I'll write the tests this evening. Thanks for the pointers 👍.

As for the docstrings, are we happy with leaving them as is and covering the issue separately? Otherwise I wrote some bullet points explaining the differences that I haven't committed yet. I don't really have a good feeling for what is too long for a narwhals docstring.

[dangotbanned]

Otherwise I wrote some bullet points explaining the differences that I haven't committed yet. I don't really have a good feeling for what is too long for a narwhals docstring.

I'm not 100% sure if you need more permissions for it, but I'd normally use a suggestion (step 7) for that kinda thing

I'm happy to take a look

[dangotbanned]

@felixgwilliams sorry for the delay, I'm hoping to review this later today 🙏

[dangotbanned]

Thanks for the PR @felixgwilliams

I've only got a few minor suggestions - I think we're pretty much good to go

[felixgwilliams]

Thanks @dangotbanned for your helpful suggestions. I'll be sure to remember the tip about comments being part of the code.

[dangotbanned]

Thanks again @felixgwilliams, welcome aboard 😉