-
-
Notifications
You must be signed in to change notification settings - Fork 147
Fix #1334: Update DataFrame.from_records signature and add tests #1335
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
Conversation
…ange np.ndarray to np_2d_array for data parameter- Change SequenceNotStr[str] to ListLike for columns and exclude parameters- Add test case with pd.Index as columns parameterAddresses review feedback from Dr-Irv
e6c054f
to
44fd08e
Compare
… use Sequence[SequenceNotStr] | Sequence[Mapping[str, Scalar]] | Mapping[str, Sequence[Scalar]]- Update columns and exclude parameters to use ListLike | None = None- Update index parameter to use SequenceNotStr for better type precisionAddresses review feedback from Dr-Irv on issue pandas-dev#1334
… np_2darray support to data parameter type annotation- Add comprehensive tests for DataFrame.from_records in test_frame.py- Fix NumPy 2.0 compatibility in test (S1 instead of a1)- Test covers np.ndarray, list of tuples, pd.Index columns, and structured arrays- Addresses GitHub issue pandas-dev#1334
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.
Be sure to test locally. Set up your environment as shown here:
https://github.com/pandas-dev/pandas-stubs/blob/main/docs/setup.md
Then do poe test
to make sure the tests pass locally.
- Update data parameter types to accept Sequence[Mapping[str, Any]] and Mapping[str, SequenceNotStr[Any]] - Add comprehensive tests for np.ndarray, tuples, and mapping inputs - Address GitHub issue pandas-dev#1334 per Dr-Irv feedback - All 207 DataFrame tests pass with no issues
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.
Make sure you are setting up the pre-commit, because your code needs to be formatted with black
.
…-dev#1334 The main changes include: - Updated data parameter types from overly restrictive Scalar to more flexible Any types - Added .reshape(2, 2) to numpy array test to handle CI compatibility issues across different numpy versions - Included a test for mapping of sequences using DataFrame constructor (which seems to be the right approach for that data type) All 207 DataFrame tests still pass
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.
Now the CI is passing, so that's good.
Still 2 issues in terms of testing DataFrame.from_records()
with a list of dicts, and just a dict.
…-dev#1334 Addresses Dr-Irv's feedback: - Updated data parameter types from restrictive Scalar to flexible Any - Added .reshape(2, 2) to numpy test for CI compatibility - Added proper dictionary tests (list and single) without tuple conversion - Added Mapping[str, Any] type support for single dictionaries - Used DataFrame constructor for mapping sequences test All tests pass.
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.
CI also failing because you are not using the pre-commit hooks and making sure that black
is used.
… values - Change index parameter type from SequenceNotStr[str] to SequenceNotStr[Hashable] - Apply black formatting to test files - Resolves CI type checking issues per Dr-Irv feedback This allows index parameter to accept integers and other hashable values, not just strings, matching pandas runtime behavior.
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.
I resolved an issue by mistake. One thing to fix in the tests.
…mapping dict test. Applied black formatting and pre-commit fixes
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.
thanks @iclectic
DataFrame.from_records()
needs better annotations #1334This PR adds a test for
DataFrame.from_records()
with tuple data to validate type annotations work correctly. The test ensures the method properly returns a DataFrame when called with a list of tuples and column names, addressing the type checking issues raised in the original issue.