-
-
Notifications
You must be signed in to change notification settings - Fork 19.1k
Zip Strict for pandas/core level files #62469 #62577
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
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 addresses issue #62434 by adding strict=True
to all zip()
calls in pandas core level files to enable "zip strict" mode. This change ensures that zip operations explicitly validate that all iterables have the same length, helping catch potential bugs where mismatched lengths could cause silent data corruption or incorrect results.
Key Changes:
- Modified all
zip()
calls in core pandas files to usestrict=True
parameter - Removed corresponding entries from the B905 exclusion list in
pyproject.toml
- One exception uses
strict=False
where intentional length mismatch is expected
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
File | Description |
---|---|
pyproject.toml | Removed B905 exclusion entries for core files now using strict zip |
pandas/core/sorting.py | Added strict=True to zip calls in sorting operations |
pandas/core/series.py | Added strict=True to zip calls in Series methods |
pandas/core/indexing.py | Added strict=True to zip calls with one intentional strict=False |
pandas/core/generic.py | Added strict=True to zip calls in generic DataFrame/Series operations |
pandas/core/frame.py | Added strict=True to zip calls with one intentional strict=False |
pandas/core/arraylike.py | Added strict=True to zip calls in array operations |
pandas/core/apply.py | Added strict=True to zip calls in apply operations |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
||
if not isinstance(indexer, tuple): | ||
indexer = _tuplify(self.ndim, indexer) | ||
|
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.
[nitpick] Consider adding a comment explaining why strict=False
is intentionally used here, as it's the only exception to the strict zip pattern in this PR.
# Intentionally use strict=False here: in some cases, indexer may be shorter or longer | |
# than self.obj.axes, and we want to ignore any extra elements rather than raise an error. | |
# This is the only exception to the strict zip pattern in this codebase. |
Copilot uses AI. Check for mistakes.
# never try to overwrite values inplace | ||
|
||
if isinstance(value, DataFrame): | ||
check_key_length(self.columns, key, value) |
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.
[nitpick] Consider adding a comment explaining why strict=False
is intentionally used here, as it deviates from the strict zip pattern being implemented throughout the codebase.
check_key_length(self.columns, key, value) | |
check_key_length(self.columns, key, value) | |
# Intentionally use strict=False: key and value.columns may differ in length, | |
# and we only want to assign for the overlapping pairs. This deviates from the | |
# usual strict zip pattern in the codebase. |
Copilot uses AI. Check for mistakes.
"pandas/tests/window/test_expanding.py" = ["B905"] | ||
"pandas/tests/window/test_rolling.py" = ["B905"] | ||
"pandas/util/_doctools.py" = ["B905"] | ||
"pandas/util/_validators.py" = ["B905"] |
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.
missed clean up in previous PR: #62540
Thanks @shivamvishal |
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.