BUG: 2D ndarray of dtype 'object' is always copied upon construction
State: closed

This change is aesthetically unpleasing, it indents too far. #26825 already suggested to separate this part into a function. Please let me know what exactly I should abstract into a function, and where to put it. Alternatively, amend this PR yourself.

Thanks for pandas. :)

- [X] closes #39263
- [X] tests added / passed
- [X] Ensure all linting tests pass, see [here](https://pandas.pydata.org/pandas-docs/dev/development/contributing.html#code-standards) for how to run them
- [X] whatsnew entry

Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
cc @jbrockmendel 
side note, after #38939 we can also avoid consolidating in this case, which would avoid making copies
From what I can gather, the problem is that not all (but some) datetimelike types are `dtype`s. Checking equality between a `dtype` and a non-`dtype` throws an exception, which is likely why this wasn't implemented in the block manager constructor (where I assume everything works in `dtype`s?)

Edit: ignore this comment
> Checking equality between a dtype and a non-dtype throws an exception

Im guessing the issue is trying to do `some_numpy_dtype == some_extension_dtype` which does raise.  if you use is_dtype_equal(some_numpy_dtype, some_extension_dtype)` you'll get a bool back
Hello @irgolic! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2021-07-14 18:45:43 UTC
> I am not sure how this change actually helps anything. e.g. how does sharing memory actually help here? (because the second you do almost anything these blocks will likely get split apart and copied again).

I'm a core developer of https://github.com/biolab/orange3, focusing on UX. Orange's data objects are also based on numpy arrays, Orange's `Table` exposes them as `table.X`, `table.Y`, `table.metas`. I'm trying to wrap our numpy arrays in DataFrame objects, and allow people to use pandas as an accessor in Table (`table.X_df`, `table.Y_df`, `table.metas_df`) (https://github.com/biolab/orange3/pull/5189).

While you're right in that most operations will split it apart anyway, in my use case, it'll avoid an unnecessary copy operation.

> you will have to measure performance if it changes in any way. run the asv suite. and see which benchmarks are affected.

On it. If none of the benchmarks change, I'll add one.
Let me know if you prefer to change `create_block_manager_from_arrays`, as is done in the latest commit, and I'll fix it up.

As the function is used elsewhere too, it makes a test fail:
```python
def test_constructor_series_copy(self, float_frame):
    series = float_frame._series
  
    df = DataFrame({"A": series["A"]})
  
    df["A"][:] = 5
  
    assert not (series["A"] == 5).all()  # False
```
So, `create_block_manager_from_arrays` won't create a copy of the series, because it's unnecessary to do so. There's only one test in pandas relying on this behavior, I'm not sure how much code around the world relies on it.
If the DataFrame contains anything more than a single series/array, it'll copy. This is how I adjusted the test too:
```python
def test_constructor_series_copy(self, float_frame):
    series = float_frame._series
  
    df = DataFrame({"A": series["A"], "B": series["A"]})
  
    df["A"][:] = 5
  
    assert not (series["A"] == 5).all()  # True
```
I think this PR will be almost trivial once https://github.com/pandas-dev/pandas/pull/38939 is merged
I've added a comment to that PR. It may or may not be trivial depending on the answer.
https://github.com/pandas-dev/pandas/pull/38939/files#r562162332

Edit: If I understand correctly, this PR complements the other.
This is necessary because of the way consolidation works, specifically the nature of `pandas.core.internals.managers._merge_blocks`:
```python
def _merge_blocks(
    blocks: List[Block], dtype: DtypeObj, can_consolidate: bool
) -> List[Block]:

    if len(blocks) == 1:
        return blocks

    if can_consolidate:

        if dtype is None:
            if len({b.dtype for b in blocks}) != 1:
                raise AssertionError("_merge_blocks are invalid!")

        # TODO: optimization potential in case all mgrs contain slices and
        # combination of those slices is a slice, too.
        new_mgr_locs = np.concatenate([b.mgr_locs.as_array for b in blocks])
        new_values = np.vstack([b.values for b in blocks])

        argsort = np.argsort(new_mgr_locs)
        new_values = new_values[argsort]
        new_mgr_locs = new_mgr_locs[argsort]

        return [make_block(new_values, placement=new_mgr_locs, ndim=2)]

    # can't consolidate --> no merge
    return blocks
```
Meaning, if you create a block for each column of the array, it won't understand that these are all slices of the same contiguous thing. To my knowledge, there's no easy way to check this. So we make sure to form one block for contiguous data.
You were right @jreback, the change can be much simpler than I wrote previously. If I were not to try to jam everything into `create_block_manager_from_arrays`, it works out quite nicely.

Still, it'd be much nicer to make a `create_block_manager_from_array` constructor. `create_block_manager_from_blocks` is now only called with a single array:
```python
def create_block_manager_from_blocks(blocks, axes: List[Index]) -> BlockManager:
    if len(blocks) == 1 and not isinstance(blocks[0], Block):
        # if blocks[0] is of length 0, return empty blocks
        if not len(blocks[0]):
            blocks = []
        else:
            # It's OK if a single block is passed as values, its placement
            # is basically "all items", but if there're many, don't bother
            # converting, it's an error anyway.
            return create_block_manager_from_array(blocks[0], axes)

    try:
        mgr = BlockManager(blocks, axes)
        mgr._consolidate_inplace()
        return mgr

    except ValueError as e:
        blocks = [getattr(b, "values", b) for b in blocks]
        tot_items = sum(b.shape[0] for b in blocks)
        raise construction_error(tot_items, blocks[0].shape[1:], axes, e)
```
Which is just an uglier way of doing this
```python
def create_block_manager_from_array(
    array,
    axes: List[Index],
    consolidate: bool = True,
) -> BlockManager:
    try:
        block = make_block(values=array, placement=slice(0, len(axes[0])), ndim=2)
        mgr = BlockManager([block], axes)
    except ValueError as e:
        raise construction_error(array.shape[0], array.shape[1:], axes, e)
    if consolidate:
        mgr._consolidate_inplace()
    return mgr
```
That's the second commit. To be clear, after this change, there are no uses of `create_block_manager_from_blocks` in pandas. If you disagree with this, let me know, and I'll remove the second commit. 
latest looks very nice. let's hope everything passes :->
:D
@irgolic can you run the construction asv's and report on the results here. to see if anything interesting. also the indexing might be useful to run (note that can take a while to run :-<)

cc @jbrockmendel if comments
Ctors asv says `BENCHMARKS NOT SIGNIFICANTLY CHANGED.`

I'll post indexing results once they're done.
```
       before           after         ratio
     [7e531e3d]       [7157e6aa]
                      <2d-object-dont-copy>
+        67.8±1μs       98.5±0.9ms  1454.32  indexing.NumericSeriesIndexing.time_getitem_scalar(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
+     6.34±0.03ms       7.06±0.2ms     1.11  indexing.NumericSeriesIndexing.time_getitem_array(<class 'pandas.core.indexes.numeric.Int64Index'>, 'unique_monotonic_inc')
-        99.8±2ms      3.66±0.04ms     0.04  indexing.NumericSeriesIndexing.time_loc_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
-        99.3±2ms          479±8μs     0.00  indexing.NumericSeriesIndexing.time_loc_slice(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
-        99.2±2ms          171±2μs     0.00  indexing.NumericSeriesIndexing.time_loc_scalar(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED.
```

I'm afraid I don't have enough knowledge to interpret these, but interesting how some rose and some dropped.
can you repeat those benchamrks to see if they repro (the ones that you showed above)
```
       before           after         ratio
     [caf81fac]       [7157e6aa]
                      <2d-object-dont-copy>
+     4.64±0.06ms          326±4ms    70.28  indexing.NumericSeriesIndexing.time_loc_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+     5.07±0.06ms          327±4ms    64.58  indexing.NumericSeriesIndexing.time_getitem_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+     12.3±0.09ms          327±4ms    26.64  indexing.NumericSeriesIndexing.time_loc_array(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+      12.6±0.5ms          329±4ms    26.03  indexing.NumericSeriesIndexing.time_getitem_array(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+      37.0±0.3ms          332±4ms     8.96  indexing.NumericSeriesIndexing.time_getitem_lists(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
-       593±200μs         527±10μs     0.89  indexing.NonNumericSeriesIndexing.time_getitem_list_like('period', 'nonunique_monotonic_inc')
-       78.2±30μs         68.3±1μs     0.87  indexing.NonNumericSeriesIndexing.time_getitem_pos_slice('period', 'nonunique_monotonic_inc')
-      5.99±0.5μs      5.19±0.05μs     0.87  indexing.DataFrameStringIndexing.time_getitem_scalar
-     3.99±0.07μs      3.44±0.08μs     0.86  indexing.NonNumericSeriesIndexing.time_getitem_scalar('string', 'non_monotonic')
-        4.18±2ms      3.34±0.02ms     0.80  indexing.InsertColumns.time_assign_list_like_with_setitem
-       745±200μs         543±10μs     0.73  indexing.NonNumericSeriesIndexing.time_getitem_list_like('datetime', 'non_monotonic')
-       585±200μs          410±2μs     0.70  indexing.AssignTimeseriesIndex.time_frame_assign_timeseries_index
-       74.1±20ms      4.16±0.09ms     0.06  indexing.NonNumericSeriesIndexing.time_getitem_list_like('period', 'unique_monotonic_inc')
-       89.8±20ms       4.50±0.2ms     0.05  indexing.NonNumericSeriesIndexing.time_getitem_list_like('period', 'non_monotonic')
-      99.6±0.3ms      3.72±0.08ms     0.04  indexing.NumericSeriesIndexing.time_loc_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
-      99.7±0.7ms         491±10μs     0.00  indexing.NumericSeriesIndexing.time_getitem_slice(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
-      98.8±0.4ms          169±3μs     0.00  indexing.NumericSeriesIndexing.time_loc_scalar(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED.
```

Was something related to indexing merged into master recently? Or are they generally a bit unreliable? I'm running these on a dedicated machine.
you shouldn't have this kind of variation 

Three more runs:

```
       before           after         ratio
     [caf81fac]       [7157e6aa]
                      <2d-object-dont-copy>
+        72.2±2μs       98.9±0.5ms  1369.02  indexing.NumericSeriesIndexing.time_getitem_scalar(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
+         163±1μs       99.3±0.5ms   607.68  indexing.NumericSeriesIndexing.time_loc_scalar(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
+         469±6μs       99.2±0.4ms   211.69  indexing.NumericSeriesIndexing.time_loc_slice(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
+         476±6μs       99.2±0.5ms   208.10  indexing.NumericSeriesIndexing.time_getitem_slice(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
+     4.62±0.04ms          327±3ms    70.80  indexing.NumericSeriesIndexing.time_loc_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+     4.96±0.06ms          327±3ms    65.98  indexing.NumericSeriesIndexing.time_getitem_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+      12.1±0.1ms          329±3ms    27.23  indexing.NumericSeriesIndexing.time_getitem_array(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+     3.66±0.04ms       99.7±0.4ms    27.21  indexing.NumericSeriesIndexing.time_loc_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
+     12.2±0.07ms          329±3ms    26.98  indexing.NumericSeriesIndexing.time_loc_array(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+      4.01±0.1ms         99.8±2ms    24.85  indexing.NumericSeriesIndexing.time_getitem_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
+      36.2±0.4ms          331±3ms     9.15  indexing.NumericSeriesIndexing.time_getitem_lists(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+      5.96±0.2ms       6.61±0.3ms     1.11  indexing.NumericSeriesIndexing.time_loc_array(<class 'pandas.core.indexes.numeric.UInt64Index'>, 'unique_monotonic_inc')
+      6.38±0.3ms       7.02±0.2ms     1.10  indexing.NumericSeriesIndexing.time_getitem_array(<class 'pandas.core.indexes.numeric.Int64Index'>, 'unique_monotonic_inc')
-        494±90μs        255±0.1μs     0.52  indexing_engines.NumericEngineIndexing.time_get_loc((<class 'pandas._libs.index.Int8Engine'>, <class 'numpy.int8'>), 'monotonic_incr')
```
```
       before           after         ratio
     [caf81fac]       [7157e6aa]
                      <2d-object-dont-copy>
+     4.77±0.04ms          322±4ms    67.53  indexing.NumericSeriesIndexing.time_loc_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+     5.02±0.04ms          325±3ms    64.61  indexing.NumericSeriesIndexing.time_getitem_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+      12.1±0.1ms          325±4ms    26.75  indexing.NumericSeriesIndexing.time_loc_array(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+      12.5±0.5ms          327±4ms    26.05  indexing.NumericSeriesIndexing.time_getitem_array(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+      36.8±0.3ms          329±3ms     8.96  indexing.NumericSeriesIndexing.time_getitem_lists(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
-     4.37±0.07ms      3.92±0.03ms     0.90  indexing.NonNumericSeriesIndexing.time_getitem_list_like('period', 'unique_monotonic_inc')
-      34.7±0.5μs       30.5±0.7μs     0.88  indexing.NonNumericSeriesIndexing.time_getitem_pos_slice('string', 'non_monotonic')
-        99.5±2ms      4.06±0.08ms     0.04  indexing.NumericSeriesIndexing.time_getitem_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
```
```
       before           after         ratio
     [caf81fac]       [7157e6aa]
                      <2d-object-dont-copy>
+         167±2μs       98.7±0.5ms   590.65  indexing.NumericSeriesIndexing.time_loc_scalar(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
+         469±8μs         99.2±2ms   211.53  indexing.NumericSeriesIndexing.time_loc_slice(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
+     4.86±0.09ms          326±3ms    67.17  indexing.NumericSeriesIndexing.time_loc_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+     5.53±0.05ms          327±3ms    59.09  indexing.NumericSeriesIndexing.time_getitem_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+     12.3±0.09ms          329±4ms    26.71  indexing.NumericSeriesIndexing.time_getitem_array(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+     12.3±0.09ms          327±3ms    26.68  indexing.NumericSeriesIndexing.time_loc_array(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
+      37.0±0.2ms          331±3ms     8.94  indexing.NumericSeriesIndexing.time_getitem_lists(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
-      10.4±0.9μs      9.42±0.07μs     0.90  indexing.CategoricalIndexIndexing.time_getitem_scalar('non_monotonic')
-      4.89±0.2ms       4.38±0.2ms     0.90  indexing.NonNumericSeriesIndexing.time_getitem_list_like('period', 'non_monotonic')
-     4.47±0.03ms      3.99±0.02ms     0.89  indexing.NonNumericSeriesIndexing.time_getitem_list_like('period', 'unique_monotonic_inc')
-         395±1μs         264±10μs     0.67  indexing_engines.NumericEngineIndexing.time_get_loc((<class 'pandas._libs.index.Int8Engine'>, <class 'numpy.int8'>), 'monotonic_incr')
-        99.5±1ms      3.78±0.08ms     0.04  indexing.NumericSeriesIndexing.time_loc_list_like(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
-        98.6±2ms       67.5±0.9μs     0.00  indexing.NumericSeriesIndexing.time_getitem_scalar(<class 'pandas.core.indexes.numeric.Float64Index'>, 'nonunique_monotonic_inc')
```
No idea where this variation comes from.
these indicate your change is impacting
my guess is that somewhere this is causing a numeric index to be upcast to object
> Or are they generally a bit unreliable? 

Yes.  Usually ill run them all once, then re-run the ones that it claims are significantly changed and prune entries that aren't consistent-ish across runs
```
> asv continuous -f 1.1 9ed521e363959ea2aa2fd5c987aa08b5730a9454 309cf3a6c76c0671ac5710e14655257c9cbcdbf8 -b indexing.NumericSeriesIndexing.time_getitem_array   
...
       before           after         ratio
     [9ed521e3]       [309cf3a6]
-         331±3ms       12.5±0.1ms     0.04  indexing.NumericSeriesIndexing.time_getitem_array(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')
SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED.
```
I could really only get this one to reproduce.

It seems that 309cf3a6 induced a performance boost. My PR undoes that performance boost.
```
> asv continuous -f 1.1 9ed521e363959ea2aa2fd5c987aa08b5730a9454 origin/2d-object-dont-copy -b indexing.NumericSeriesIndexing.time_getitem_array  
...
BENCHMARKS NOT SIGNIFICANTLY CHANGED.
```
```
> asv continuous -f 1.1 309cf3a6c76c0671ac5710e14655257c9cbcdbf8 origin/2d-object-dont-copy -b indexing.NumericSeriesIndexing.time_getitem_array
...
       before           after         ratio
     [309cf3a6]       [7157e6aa]
+      12.5±0.2ms          332±3ms    26.69  indexing.NumericSeriesIndexing.time_getitem_array(<class 'pandas.core.indexes.numeric.Float64Index'>, 'unique_monotonic_inc')

SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY.
PERFORMANCE DECREASED.
```
@jbrockmendel Any idea why?
I just needed to rebase. 
```
> asv continuous -f 1.1 upstream/master origin/2d-object-dont-copy -b indexing.NumericSeriesIndexing.time_getitem_array
...
BENCHMARKS NOT SIGNIFICANTLY CHANGED.
```
We're good to go.

You guys squash your merges, right? I don't need to squash my commits?
This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.
This PR is waiting for #38939, I assume.
> This PR is waiting for #38939, I assume.

@irgolic #38939 is merged. can you merge master to resolve conflicts
@simonjayhawkins merged, could you approve running lint and tests?

some changes have been made in dataframe constructors and block manager constructors since I wrote this. at a glance, dataframe constructor tests pass, and it should be fine, I'm rerunning asv though
> @simonjayhawkins merged, could you approve running lint and tests?

done