-
Notifications
You must be signed in to change notification settings - Fork 97
Closes #5175: overloads for in1d #5341
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
0895217 to
3ba7c42
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #5341 +/- ##
========================================
Coverage ? 100.00%
========================================
Files ? 5
Lines ? 115
Branches ? 0
========================================
Hits ? 115
Misses ? 0
Partials ? 0
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
efecf86 to
7ca62f6
Compare
d65ee9b to
2935faa
Compare
2935faa to
5d43cab
Compare
| f"Mapping MultiIndex has {len(mkeys)} levels but GroupBy has {len(gb_keys)} keys" | ||
| ) | ||
|
|
||
| mask = in1d(gb_keys, mkeys, invert=True) |
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.
It might be good to have a comment that invert=True means mask identifies original keys that are not specified in the mapping (i.e. the missing values).
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.
Does this look better? 330c5c7
ajpotts
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.
Nice work.
This PR improves the behavior, typing, and correctness of
in1dand propagates those fixes and capabilities through pandas-style APIs that depend on it (notablySeries.isin,Series.map, and joins).Key changes
1.
in1dsymmetric + invert correctnessAdds precise overloads and return types for
symmetric=TruevsFalse.Fixes a bug where
invert=Truewas only applied to one side of the symmetric result.Cleans up logic so
invertis applied consistently in bothassume_uniqueand non-unique paths.Updates docstring to clearly specify:
symmetric=False→ single mask forAsymmetric=True→(maskA, maskB)with correct semanticsAdds comprehensive tests covering:
assume_uniqueon/offsymmetricon/offinverton/off2. Type system improvements
Introduces
groupabletyping and overloads soin1dis properly typed for:Fixes downstream mypy issues by:
in1dSeries.isinandSeries.__setitem__MultiIndexmembership explicitly vialookup3. MultiIndex-safe
Series.mapReworks
mapto:Index/MultiIndexNaNand string/categorical with"null"Adds fast-path for empty mappings.
Ensures no tuple/scalar mixing when computing missing keys.
Adds tests validating correct behavior for MultiIndex + missing keys.
4. API robustness
Series.isinnow:listandtupleinputsSeries.__setitem__:Overall impact
This PR:
in1d(symmetric=True, invert=True).in1dfully well-typed and self-consistent.in1dwith MultiIndex, GroupBy, joins, and Series operations.Closes #5175: overloads for in1d