[ArrayManager] DataFrame constructors

[jorisvandenbossche]

xref #39146

In this PR, I am trying to ensure what when we construct a DataFrame, we directly construct the proper manager (depending on the option settings) instead of converting at the end of the constructor (as is done now). To that end, I am passing through a keyword to indicate the type of the manager to be created to the different internal construction methods like arrays_to_mgr etc.

[jorisvandenbossche]

@jbrockmendel other comments?

[jbrockmendel]

lgtm cc @jreback

[jreback]

pls rebase as well

[jreback]

can you rename typ -> manager?

[jorisvandenbossche]

Note this is the same keyword as I use for arrays_to_mgr, mgr_to_mgr, etc, where I think the typ keyword makes sense (it's already clear from the name of the function that it is creating a manager).

Alternatively, we could make the init_dict function name more explicit (and consistent with the others) and eg rename to dict_to_mgr. I am happy to do this in a separate PR.

[jreback]

ok yeah +1 on the rename (followon ok ) and anything to make the keyword more obvious

[jorisvandenbossche]

-> #40074

[jorisvandenbossche]

@jreback more answers / comments here?

[jbrockmendel]

could do this unconditionally. tradeoff: slightly less complex code here, perf of unecessary wrapping/unwrapping in DatetimeBlock/TimedeltaBlock._maybe_coerce_values (for now, until upcoming PR)

[jorisvandenbossche]

I would then leave that until it is useful

[jreback]

can you do this inside e.g. in ArrayManager constructor (or add a constructor on L127), prob should do that anyhow.

[jorisvandenbossche]

add a constructor on L127

What do you mean exactly with that?
We already use the main constructor ArrayManager(arrays, [index, columns]) on L127 because that constructor already works from "arrays" (in constrast to the BlockManager, which needs to consolidate arrays etc, so therefore there is a helper create_blockmanager_from_arrays, but for ArrayManager this is exactly the __init__ already)

[jbrockmendel]

i think the suggestion is to move the call on L115 to just before the return ArrayManager on L127. i think thatd be a small improvement, not a big deal

[jorisvandenbossche]

I intentionally put it here because, in theory, I think this wrapping only needs to be done with the default of verify_integrity=True (so you have a way to skip it)

[jreback]

I mean on line L127 this should call a method inside internal and not directly calling the manager. Then you can handle the special cases. on L110 inside that function.

[jorisvandenbossche]

So you want me to write this function?

def create_array_manager_from_arrays(arrays, axes, verify_integrity=True):
    if verify_integrity:
        arrays = [ensure_wrapped_if_datetimelike(arr) for arr in arrays]
    return ArrayManaget(arrays, axes)

(IMO this just splits the handling of verify_integrity to multiple places (as mentioned above, that's the reason that I put this wrapping where it is located now), but I don't care if this makes you happy)

this should call a method inside internal

Note you are commenting on a file that already is inside internals

[jreback]

its L110 which needs to be handled in array_manager.py and not here

[jorisvandenbossche]

OK, moved inside array_manager.py

[jreback]

can you do this inside e.g. in ArrayManager constructor (or add a constructor on L127), prob should do that anyhow.

[jorisvandenbossche]

All good here? (this is blocking a few other PRs that are mostly ready: #39700, #40050)

[jreback]

need a rebase

[jreback]

I mean on line L127 this should call a method inside internal and not directly calling the manager. Then you can handle the special cases. on L110 inside that function.

[jorisvandenbossche]

Moving the datetime wrapping into the ArrayManager constructor revealed some other issues, so first doing a precursor for this here -> #40147

[jorisvandenbossche]

#40147 is merged, so updated this (I think the only remaining discussion item was about where the datetime wrapping happened, which is resolved by #40147)

[jreback]

lgtm.