ENH: 2D support for MaskedArray
State: closed

This doesn't in any way _use_ the 2D support, but opens up the option of incrementally fleshing out the tests.

Labels:
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
> Such a significant architectural change shouldn't be merged without prior discussion

sure, what are your concerns
The ExtensionArrays have been 1D from the start (for around 3 years now). And idem for MaskedArray in specific. So if Brock proposes to change that, then I think it is to start up to the proposer to come up with arguments for changing this. 

There has been discussion before about 1D vs 2D extension arrays, for sure (although I don't think any of that discussion resulted in a clear decision to merge a PR like this). But specifically for MaskedArray, we didn't have any discussion about this, AFAIK. I think that at least requires *some* discussion about whether we want this or not? 

The masked arrays have been explicitly designed to be 1D, of course also because currently ExtensionArrays in general are 1D, but in addition, there are several ideas for future improvements (https://github.com/pandas-dev/pandas/issues/30435), exploring bitmask instead of boolean mask (https://github.com/pandas-dev/pandas/issues/31293), optionally using arrow under the hood like we are doing for the string array, more efficient zero-copy conversion with arrow, nested data types (eg https://github.com/pandas-dev/pandas/issues/35176). Those are all not impossible with 2D arrays, but IMO will be much easier with 1D arrays, and thus requires some consideration.

It's also not fully clear to me where this PR is going towards. At the moment it is adding quite some complexity for something that is not yet used. What's the plan for how to actually use it in pandas? Do we want to give the masked arrays 2D capabilities (so this ability can be used for certain operations), but keep storing them as 1D in DataFrames? Or do we want to change the ExtensionBlock to a consolidated 2D Block? But only for masked arrays, or for all ExtensionArrays? What for arrays that cannot easily be 2D (eg nested array)? What's the idea for externally defined ExtensionArrays? ...
The reason to do this is roughly the same reason why we're moving forward with ArrayManager: so that we can see if actually using this is something we want to do longer-term.
gentle ping, plenty more tests where these came from
ok I am +1 on merging this. I agree with @jbrockmendel reasoning here. We *don't* really know where we are going to ultimately go, e.g. ArrayManger or simplified BlockManager. We need more support & performance testing to see. Sure I'd like to see a unified approach, but we have advocates for both and would rather not inhibit experimentation.

cc @jorisvandenbossche 
> The reason to do this is roughly the same reason why we're moving forward with ArrayManager: so that we can see if actually using this is something we want to do longer-term.

For the simplified non-consolidating BlockManager, I started with a description of arguments for it, we had an extensive discussion about it, with several people expressing their interest for it, and with the main question mark being performance. At which point we need a proof of concept to test things. 

As far as I know, we have had no such discussion about 2D masked arrays. 


> As far as I know, we have had no such discussion about 2D masked arrays.

We've had the same discussion about 2D EAs repeatedly.
@jorisvandenbossche do you have actual concrete objections to merging this? We are allowing ArrayManager on an experimental basis, I don't see how this is any different.
I think my longer comment above (https://github.com/pandas-dev/pandas/pull/38992#issuecomment-757026174) already includes some concrete concerns. Reformulating them:

* We have had many discussions about 2D ExtensionArrays, yes (mostly in 2019, see eg https://github.com/pandas-dev/pandas/pull/27142 and linked PRs and the mailing list discussion at https://mail.python.org/pipermail/pandas-dev/2019-June/000983.html), but AFAIK those discussions have not yet led to a consensus or compromise in favor of 2D EAs (if I recall correctly, the use of 2D arrays for datetime ops was discussed then as a compromise). 
  Fully supporting 2D ExtensionArrays is a big change, that requires a more detailed proposal and discussion IMO. And we already have the existing consolidating BlockManager to know how internals with 2D arrays would work (and to compare the ArrayManager with).
* IMO this PR is missing context on how we would actually use this in pandas. I think we should at least have some idea about that before merging this (I asked several questions above (eg do we want to make ExtensionBlock 2D? What does this mean for other EAs? ...), to which no response has been given)
* The POC for the ArrayManager is mostly independent from the existing code (eg the BlockManager didn't become any more complex due to merging it), while this is profoundly changing the existing MaskedArrays, making it more difficult to further improve them as 1D arrays (there is still a lot of work to make them fully feature-complete to start with, and I mentioned several possible additional enhancements above)

(having a call about this might help resolve some of those discussion points?)

I'm tired of repeating myself.  At the sprint in 2019 we (including Wes) agreed to move forward with 2D EA support for experimentation.  Since then the only thing we've learned is that you are more willing to repeat the same arguments over and over again than I am, and everyone else makes the entirely reasonable decision to tune it out.

> there is still a lot of work to make them fully feature-complete to start with

I would dearly like to see that, which I see as part-in-parcel with the fix-many-xfails that I brought up on last week's call.  But I don't see any effort towards making them happen, or any reason why they are mutually exclusive with this experimentation.
id like to revisit this.  this would make it feasible to experiment with/demonstrate all EAs supporting 2D.
needs a rebase too
> is there perf impact on BM? on AM? (note on AM it might make sense to actually put arrays together and hit a single 2-d function / or have a fast-path inside the function itself).

The perf impact should be neglible.  MaskedArray reductions will have an extra `if axis == 0` check that will always evaluate to True (bc as mentioned in the OP, this doesn't actually _use_ the 2D MaskedArrays outside of tests)
@jbrockmendel if you want to revisit this, can you then respond to my comments above? You said that you "are tired of repeating yourself", but I asked very concrete questions about this PR to which I didn't see any response (see eg second bullet point at https://github.com/pandas-dev/pandas/pull/38992#issuecomment-765441069 or last paragraph at https://github.com/pandas-dev/pandas/pull/38992#issuecomment-757026174)

> At the sprint in 2019 we (including Wes) agreed to move forward with 2D EA support for experimentation.

I don't remember exactly what was said during the sprint, but I know that afterwards you and Tom experimented with several approaches to support both 1D and 2D EAs (metaclass, composition, .. see eg https://github.com/pandas-dev/pandas/pull/27142, https://github.com/pandas-dev/pandas/pull/28389), and in the end we decide to not pursue general 2D EAs (but focus on datetimelike ops in 2D) because of the difficulties related to that. So that's the reason I am wondering what the next steps would be now.

In 2019, we also didn't have that many masked arrays (only the integer array was there in an initial version). As I mentioned above, there are good reasons to keep masked arrays specifically as 1D (which don't necessarily apply to EAs in general). 

> this would make it feasible to experiment with/demonstrate all EAs supporting 2D.

Can you explain what you exactly want to demonstrate? The masked arrays are fully optional and always the alternative of another default dtype. So you should be able to demonstrate eg a working pandas with 2D EAs without having to involve 2D masked arrays? 
Just like we are implementing ArrayManager to experiment with an all-1D world, this is so that we can experiment with an all-2D world.

> Can you explain what you exactly want to demonstrate?

I want to 1) check that I can get all the tests passing, 2) measure performance, 3) measure (eyeball) complexity in a world where all our EAs support 2D.

> So you should be able to demonstrate eg a working pandas with 2D EAs without having to involve 2D masked arrays?

Not if I want to get the test suite passing and run all the asvs.

> [https://github.com/pandas-dev/pandas/pull/38992#issuecomment-757026174...] Those are all not impossible with 2D arrays, but IMO will be much easier with 1D arrays.

Irrelevant.  Because this doesn't _use_ 2D MaskedArray outside of dedicated tests, the hypothetical features you list would not need to support 2D.  If we merge this and later have a genuinely-1d-only feature we want to merge, this can be reverted.

> [https://github.com/pandas-dev/pandas/pull/38992#issuecomment-757026174] At the moment it is adding quite some complexity for something that is not yet used

Eye of the beholder I guess.  Most of it is just passing `axis=axis` around.  Is there something in particular you find unclear I can try to simplify?

> [https://github.com/pandas-dev/pandas/pull/38992#issuecomment-765441069] The POC for the ArrayManager is mostly independent from the existing code (eg the BlockManager didn't become any more complex due to merging it), while this is profoundly changing the existing MaskedArrays

Same thing, I just don't see anything about this as being that profound.  And while the ArrayManager itself doesn't complicated BlockManager, it has complicated the tests and frame.py, internals.construction, ... and I don't mind, because it is serving a purpose of letting us experiment.  Just like this.

> [https://github.com/pandas-dev/pandas/pull/38992#issuecomment-757026174] What's the plan for how to actually use it in pandas? 

Short-term: nothing user facing.  For this and the next few hypothetical PRs that would follow, I just want to flesh out the test suite and get everything passing.

Longer-term: if we go down this road then all EAs support 2D.  A 2D ExtensionBlock would be directly backed by a 2D EA.

> [https://github.com/pandas-dev/pandas/pull/38992#issuecomment-757026174] What for arrays that cannot easily be 2D (eg nested array)?

I don't understand a) why a nested array couldn't be 2D or b) why this is pertinent to this PR.

> [https://github.com/pandas-dev/pandas/pull/38992#issuecomment-757026174] What's the idea for externally defined ExtensionArrays? 

If we decide on an all-2D world (which this PR does not commit us to doing) then there would be a deprecation cycle for downstream authors.  But this is getting way ahead of ourselves.
@jbrockmendel these changes are entirely internal? removing milestone.
> these changes are entirely internal?

yes
gentle ping; this would simplify a corner case in #33036
ok tests are failing.

i suppose it makes sense to support both 1d and 2d kernels on things. it does lead to some code duplication, but performance can be great if we don't need to operate column-by-column all the time. However the codebase is *mostly* 1d currently, with some efforts to add 2d kernels.

does this concur with your thinking?
> ok tests are failing.

Fixed

> [...] does this concur with your thinking?

Was this part of the comment supposed to go in #42841?  Will answer there.

@jbrockmendel needs rebase
certainly fine for testing things out. thanks @jbrockmendel 

a couple of followups
@simonjayhawkins addressing your post-merge comments has been on my todo list for a while, looks likely to fall off.  Do they merit their own Issue?