-
-
Notifications
You must be signed in to change notification settings - Fork 154
fix: fix DataFrame.from_records with data as an iterator #1376
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
base: main
Are you sure you want to change the base?
fix: fix DataFrame.from_records with data as an iterator #1376
Conversation
|
Hi @terencehonles , thank you for the proposal. However, according to the Pandas documentation, the |
|
I'm not sure if I have time to follow up right now. I opened this PR because mypy is now complaining about some existing code I have (which I have silenced with an ignore statement). This is the pandas implementation and it does handle an iterator https://github.com/pandas-dev/pandas/blob/f6fa9b679006d08fa99d8c2b6dbd834ec60875d6/pandas/core/frame.py#L2240-L2263. |
|
@terencehonles the docs are updated, so now @cmp0xff can review this PR and provide any feedback |
cmp0xff
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The changes are now in accordance with the Pandas documentation.
One minor comment: the idea in L4702-L4711 seems to be duplicated by the code snippet below. Could you remove the latter? Thanks.
pandas-stubs/tests/test_frame.py
Lines 4730 to 4739 in 2c603f6
| # Testing with list of tuples (instead of structured array for type compatibility) | |
| data_array_tuples = [(1, "a"), (2, "b")] | |
| check( | |
| assert_type( | |
| pd.DataFrame.from_records(data_array_tuples, columns=["id", "name"]), | |
| pd.DataFrame, | |
| ), | |
| pd.DataFrame, | |
| ) |
|
Hi @terencehonles , could you make the small change mentioned in #1376 (review)? |
Removed test case for list of tuples in DataFrame assertions.
cmp0xff
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Almost there!
| | Sequence[SequenceNotStr] | ||
| | Sequence[Mapping[str, Any]] | ||
| | Iterator[SequenceNotStr] | ||
| | Iterator[Mapping[str, Any]] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sequence is a subset of Iterator. Also the type variable in SequenceNotStr does not have a default value, so pyright-strict would complain.
| | Sequence[SequenceNotStr] | |
| | Sequence[Mapping[str, Any]] | |
| | Iterator[SequenceNotStr] | |
| | Iterator[Mapping[str, Any]] | |
| | Iterator[SequenceNotStr[Any]] | |
| | Iterator[Mapping[str, Any]] |
| pytest = ">=8.4.2" | ||
| pyright = ">=1.1.407" | ||
| ty = ">=0.0.1a24" | ||
| ty = "0.0.1a25" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Newer version has been released
| ty = "0.0.1a25" | |
| ty = ">=0.0.1a26" |
assert_type()to assert the type of any return value