BUG: #10445 cannot add DataFrame to empty Panel

[Garrett-R]

closes #10445

I based this bugfix off how DataFrame handles the analogous situation ‒ being intialized to empty and then having a series added (cf. DataFrame._ensure_valid_index)

[jreback]

a more generic fix to this would be nice. IOW, put rename DataFrame._ensure_valid_index to _ensure_valid_axes and put in core/generic/NDFrame (and then DataFrame/Panel.__setitem__ can call it; Series doesn't need it).

then you can use the properties of the object to make sure that all of the not info_axis are valid

you iterate over the .axes (and skip the info_axis). For a frame this will yield (0), and a panel (1,2)

[Garrett-R]

Right, good call. I'll do that this week.

[Garrett-R]

@jreback, so, there might an issue with merging the functionality of both DataFrame's _ensure_valid_index and Panel's _ensure_valid_major_minor_axes (which was part of this PR).

The DataFrame needs to case the incoming value to Series while the Panel one needs to cast to DataFrame.

At first I was worried about introducing a circle import, but in fact, I see generic.py is already circularly importing pandas.core.frame (through the import pandas as pd line). I guess I could special case the two options, but that would be kind of convoluted right? If I'm special casing for the 2 cases, I might as well keep the original two functions separate.

[jreback]

use _constructor_sliced that's what its there for. We rarely want to special case if at all possible.

[Garrett-R]

Hmmm... I'm struggling with this one.

Attempt 2 to move this into NDFrame is no dice. You said above I could use the non-info axes to reindex and "For a frame this will yield (0), and a panel (1,2)". But actually, the axes I need to reindex is (1) for a DataFrame and (1,2) for a Panel...

Now that would not be hard to do by just doing a for loop and starting from 1 and incrementing up, but that seems like a convoluted way to tackle this.

Any ideas? Moving the functionality up to the base class might save some lines of code and is certainly clever, but keeping the reindexing in the derived classes wins in terms of simpleness. Am I missing an advantage of moving to the base class?

[jreback]

@Garrett-R can you update according to above comments.

[Garrett-R]

@jreback, I was kind of struggling with moving the functionality up to the base class as per my last comment. I have time to keep trying (EDIT: meant to say: "I don't have"). If you think it's better to have the more general solution, we should probably close this PR and let the next person do it.

[jreback]

@Garrett-R will leave this open if you find the time to come back to it.

[jreback]

closing for now, if you want to update according to comments, pls reopen

[Garrett-R]

@jreback, I apologize for not responding. I probably won't end up getting back to it. Sorry and thanks for your time!