Skip to content

REF: Share NumericArray/NumericDtype methods #45997

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

Merged
merged 5 commits into from
Feb 16, 2022

Conversation

jbrockmendel
Copy link
Member

  • closes #xxxx (Replace xxxx with the Github issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

@@ -41,6 +44,22 @@

class NumericDtype(BaseMaskedDtype):
_default_np_dtype: np.dtype
_checker: Callable[[Any], bool] # is_foo_dtype
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd be more partial to _dtype_checker, but won't die on that hill

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure. OK for follow-up?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup sounds good

Copy link
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Off-topic question: I am interested in exploring the feasibility of adding an Arrow-back NumericArray. It doesn't appear that I can just subclass this since NumericArray and it's subclass are heavily numpy based.

Any recommendations on where to start? Would I need to essentially create MaskedArrowArray and go from there?

@jbrockmendel
Copy link
Member Author

Any recommendations on where to start? Would I need to essentially create MaskedArrowArray and go from there?

I've been thinking we should refactor as much of ArrowStringArray as possible out into a general arrow-backed EA. Not sure if that could be a fully-formed EA or if it would need to be a mixin. (also in the tests there's an ArrowExtensionArray but I think its half-baked)

FWIW my current thought with MaskedArray is to try to get as much of NumericArray and BooleanArray pushed down into MaskedArray as possible so it can just become a wrapper around an arbitrary non-masked array (including e.g. other EAs)

@mroeschke mroeschke added NA - MaskedArrays Related to pd.NA and nullable extension arrays Refactor Internal refactoring of code labels Feb 15, 2022
@mroeschke mroeschke added this to the 1.5 milestone Feb 15, 2022
Copy link
Member

@mroeschke mroeschke left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the gameplan. LGTM.

@jreback jreback merged commit cf2dfa7 into pandas-dev:main Feb 16, 2022
@jreback
Copy link
Contributor

jreback commented Feb 16, 2022

luv it

@jbrockmendel jbrockmendel deleted the ref-masked branch February 16, 2022 15:45
yehoshuadimarsky pushed a commit to yehoshuadimarsky/pandas that referenced this pull request Jul 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NA - MaskedArrays Related to pd.NA and nullable extension arrays Refactor Internal refactoring of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants