-
-
Notifications
You must be signed in to change notification settings - Fork 155
type multiindex constructors #1126
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
| def from_arrays( | ||
| cls, arrays, sortorder=..., names: SequenceNotStr[Hashable] = ... | ||
| cls, | ||
| arrays: SequenceNotStr[SequenceNotStr[Hashable] | AnyArrayLike], | ||
| sortorder: int | None = ..., |
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.
This one's a bit odd
The name is from_arrays, and the pandas docstring says that this should be a sequence of array-like
But, the pandas docs (and tests) have several examples of passing lists, which are not ArrayLike
lists are not included in AnyArrayLike, so I've used SequenceNotStr[Hashable] | AnyArrayLike
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.
🤔 maybe ListLike would be more suitable
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 think it should be Sequence[Axes] because this works:
>>> pd.MultiIndex.from_arrays([range(2), [3,4]], names=["a", "b"])
MultiIndex([(0, 3),
(1, 4)],
names=['a', 'b'])
>>> pd.MultiIndex.from_arrays([{1:"x", 2:"y"}, [3,4]], names=["a", "b"])
MultiIndex([(1, 3),
(2, 4)],
names=['a', 'b'])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.
🤔 maybe
ListLikewould be more suitable
Problem is that ListLike allows a single string, so we can't use it. I think Axes is the right choice.
| def from_arrays( | ||
| cls, arrays, sortorder=..., names: SequenceNotStr[Hashable] = ... | ||
| cls, | ||
| arrays: SequenceNotStr[SequenceNotStr[Hashable] | AnyArrayLike], | ||
| sortorder: int | None = ..., |
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 think it should be Sequence[Axes] because this works:
>>> pd.MultiIndex.from_arrays([range(2), [3,4]], names=["a", "b"])
MultiIndex([(0, 3),
(1, 4)],
names=['a', 'b'])
>>> pd.MultiIndex.from_arrays([{1:"x", 2:"y"}, [3,4]], names=["a", "b"])
MultiIndex([(1, 3),
(2, 4)],
names=['a', 'b'])|
thanks for your review and explanations! |
Dr-Irv
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.
thanks @MarcoGorelli
assert_type()to assert the type of any return value