-
-
Notifications
You must be signed in to change notification settings - Fork 18.8k
API: make construct_array_type non-classmethod #62060
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
@@ -150,7 +150,7 @@ def _coerce_to_data_and_mask( | |||
if dtype is not None: | |||
dtype = dtype_cls._standardize_dtype(dtype) | |||
|
|||
cls = dtype_cls.construct_array_type() | |||
cls = dtype_cls().construct_array_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.
Nit: Is it possible to change _coerce_to_data_and_mask
to pass an dtype instance 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.
thats slightly more invasive since it means updating the annotations etc
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.
OK good for a follow up then. Would like to avoid always assuming here that it's safe to instantiate these types with no arguments
Co-authored-by: Matthew Roeschke <[email protected]>
Thanks @jbrockmendel |
Co-authored-by: Matthew Roeschke <[email protected]>
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.Makes the method robust to the possibility of keywords (e.g. na_value, storage) that determine what EA subclass you get. StringDtype already does this.