ENH: Make DataFrame generic
#1566
Conversation
cmp0xff
force-pushed
the
feature/make-dataframe-generic
branch
from
6c3b2d5 to
c8d80ef
Compare
loicdiridollou
left a comment
loicdiridollou
left a comment
There was a problem hiding this comment.
I am not sure I see the whole idea about this PR, also I would have expected more breakages in the code, like for transpose method it should flip the index and columns
| @overload | ||
| def __new__( | ||
| cls, | ||
| data: DataFrame[IndexT0, IndexStrT0], |
There was a problem hiding this comment.
Not totally sure if this is gonna break a lot of things, by default DataFrame will have RangeIndex as index and columns, isn't this gonna change that?
There was a problem hiding this comment.
This is copying another DataFrame, I suppose, so nothing is changed. Will add tests later.
There was a problem hiding this comment.
If you create a DataFrame from data without an index, RangeIndex will surely take over. That takes further overloads of __new__.
As for now I just want to show a prototype of what can happen.
|
|
||
| if TYPE_CHECKING: # noqa: PYI002 | ||
| IndexT0 = TypeVar("IndexT0", bound=Index, default=Index) | ||
| IndexStrT0 = TypeVar("IndexStrT0", bound=Index, default=Index[str]) |
There was a problem hiding this comment.
Shouldn't the default be RangeIndex here?
There was a problem hiding this comment.
For df.columns we somehow have a preference for it being Index[str], see def columns(self) -> Index[str]: ... in the old code. That's the reason.
cmp0xff
left a comment
cmp0xff
left a comment
There was a problem hiding this comment.
transpose method it should flip the index and columns
Can certainly do, if the whole idea is viable.
More importantly, we need to figure out if _LocIndexFrame can be fixed at all, before continuing.
Last time when I was trying to add the backend type variable to Series, too many things crashed so that I could not continue. This time it seems more controllable for now.
| @overload | ||
| def __new__( | ||
| cls, | ||
| data: DataFrame[IndexT0, IndexStrT0], |
There was a problem hiding this comment.
This is copying another DataFrame, I suppose, so nothing is changed. Will add tests later.
|
|
||
| if TYPE_CHECKING: # noqa: PYI002 | ||
| IndexT0 = TypeVar("IndexT0", bound=Index, default=Index) | ||
| IndexStrT0 = TypeVar("IndexStrT0", bound=Index, default=Index[str]) |
There was a problem hiding this comment.
For df.columns we somehow have a preference for it being Index[str], see def columns(self) -> Index[str]: ... in the old code. That's the reason.
| @overload | ||
| def __new__( | ||
| cls, | ||
| data: DataFrame[IndexT0, IndexStrT0], |
There was a problem hiding this comment.
If you create a DataFrame from data without an index, RangeIndex will surely take over. That takes further overloads of __new__.
As for now I just want to show a prototype of what can happen.
I'm in agreement. If this were to work, what would be the benefit? Note that |
3 participants
Towards #1548
setterwork so that the typings can be updated when one assignsindexorcolumns.__getitem__in_LocIndexerFrameneed to be fixedComments and suggestions are welcomed.