-
-
Notifications
You must be signed in to change notification settings - Fork 160
TST: xStringArray and NumpyExtensionArray
#1594
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
TST: xStringArray and NumpyExtensionArray
#1594
Conversation
pandas-stubs/core/arrays/base.pyi
Outdated
| def value_counts( | ||
| self, dropna: bool = True | ||
| ) -> Series[int]: ... # probably to put in base class |
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.
In NDArrayBackedExtensionArray instead of here.
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.
Are we sure about this, seems like ExtensionArray does not inherit from any other class so wondering how the signature would percolate?
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.
My stand is that either we keep the original inheritance, or we can omit some classes if there is no public API. value_counts is public, and lives in NDArrayBackedExtensionArray, so we should keep it there.
loicdiridollou
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.
Minor questions, will do a second pass tomorrow since it is a rather long PR.
|
Indeed it is too big, will create a few smaller PRs before the main one |
9d39019 to
5af2d1f
Compare
5af2d1f to
976490a
Compare
…e/numpy-extension-array
…e/numpy-extension-array
PandasArray -> NumpyExtensionArrayxStringArray and NumpyExtensionArray
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 @cmp0xff
* fix: nightly * confirm pandas-dev/pandas#63567 * formatting
Towards #1589, #1469, #1580
A
NumpyExtensionArrayis produced