-
-
Notifications
You must be signed in to change notification settings - Fork 156
Add variance checks to aggregation tests and type assertions #1546
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?
Conversation
- Updated aggregation tests in `test_agg.py` to include checks for the `var()` method across different data types (float, bool, int, and complex). - Added a warning check for complex series when calculating variance to handle potential loss of imaginary parts. - Enhanced type assertions in `test_series.py` for the `var()` method to ensure it returns a float type.
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.
Pull request overview
This PR adds comprehensive variance checks to aggregation tests and enhances type assertions for the var() method across different data types (float, bool, int, and complex). The PR addresses issue #1545 by ensuring the variance method has proper type stubs and test coverage.
- Added type assertions using
assert_type()andcheck()forvar()method calls in type checking tests - Added variance test coverage across different data types with proper warning checks for complex numbers
- Updated type stubs with overloaded signatures for the
var()method to return appropriate types based on series dtype
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/series/test_series.py | Enhanced test_types_var() with type assertions to verify var() returns float type |
| tests/series/test_agg.py | Added var() checks across all aggregation tests for different data types (float, bool, int, complex, str, timedelta) with appropriate warning handling for complex series |
| tests/_typing.py | SHOULD NOT BE COMMITTED - This is an auto-generated file during test runs per tests/conftest.py |
| pandas-stubs/core/series.pyi | Added overloaded type signatures for var() method with specific return types for different series types |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Since you have clicked If I used AI to develop this pull request, I prompted it to follow AGENTS.md., please also follow the section https://github.com/pandas-dev/pandas-stubs/blob/main/AGENTS.md#pull-requests-summary
| ddof: int = 1, | ||
| numeric_only: _bool = False, | ||
| **kwargs: Any, | ||
| ) -> np.float64: ... |
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.
| ) -> np.float64: ... | |
| ) -> float: ... |
| check(assert_type(series.mean(), pd.Timedelta), pd.Timedelta) | ||
| check(assert_type(series.median(), pd.Timedelta), pd.Timedelta) | ||
| check(assert_type(series.std(), pd.Timedelta), pd.Timedelta) | ||
| # Note: var() is not supported for Timedelta series at runtime |
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.
Please use if TYPE_CHECKING_INVALID_USAGE and make sure series.var() here gives either errors by type checkers (like in test_agg_str) or Never (assert_type(series.var(), Never)).
Error is preferable over Never.
|
|
||
| check(assert_type(series.mean(), pd.Timestamp), pd.Timestamp) | ||
| check(assert_type(series.median(), pd.Timestamp), pd.Timestamp) | ||
| check(assert_type(series.std(), pd.Timedelta), pd.Timedelta) |
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.
Please use if TYPE_CHECKING_INVALID_USAGE and make sure series.var() here gives either errors by type checkers (like in test_agg_str) or Never (assert_type(series.var(), Never)).
Error is preferable over Never.
| check(assert_type(series.std(), np.float64), np.float64) | ||
| with pytest_warns_bounded( | ||
| np.exceptions.ComplexWarning, | ||
| r"Casting complex values to real discards the imaginary part", |
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.
please use upper=2.3.99
Updated aggregation tests in
test_agg.pyto include checks for thevar()method across different data types (float, bool, int, and complex).Added a warning check for complex series when calculating variance to handle potential loss of imaginary parts.
Enhanced type assertions in
test_series.pyfor thevar()method to ensure it returns a float type.Closes Scalar type to float error with pyright #1545 (Replace xxxx with the Github issue number)
Tests added (Please use
assert_type()to assert the type of any return value)If I used AI to develop this pull request, I prompted it to follow
AGENTS.md.