-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Deprecate pd.*Index*(names='') #19295
Comments
its easiest just to only accept SO Proposal 1. 2b |
I like this!
I don't like this! You mean in a 1-level |
yeah that’s fine ; let’s make it explicit |
What about |
By the way, the more I think about it, the more my "Proposal 2" looks cleaner to me. But it's more a vague sensation than an objective assessment. |
I think we should look back here at what is the goal? Why do we need/want compatibility? Is it because people want to be able to write code to creates an index without knowing in advance the values and names it receives will contains tuples or not / will result in MultiIndex or not? I think that is a legitimate case, but, one that is only relevant for But that would also lead me to conclue: for the single level Index subclasses (Int64Index, DatetimeIndex, ...) this will never be the case, so for those we don't care and we use the 'normal' keyword, i.e. So I think this is more one of the options of Proposal 1? Can you explain more what you find attractive in Proposal 2? It feels very strange to me to see |
@jorisvandenbossche you are right that (in my proposal 2) the support for This said, I think (although I don't think it is a priority) that
idx = Index(some_data_which_I_ignore, names=tuple_with_maybe_1_element)
idx2 = idx.copy(names=another_tuple_with_maybe_1_element) ... and hence (presumably) also in the constructor
But again, I'm relatively indifferent on this. |
It might well be that it is not difficult to support it, but still, what's the point to support it?
That could be interesting (not sure if it is needed, you can also do .copy().rename()), but I don't think that necessarily means it should be the same in the constructor.
"where applicable", yes, and
Me as well, as indeed, we can also simply allow it and convert it under the hood, without advertising it. But, if we allow it, it should be documented, and that will add to the docstring for something that is not really needed IMO. |
So I would vote for: Index subclasses only |
cc @PoppyBagel |
Pushing to the next release. |
possibly closed by #38597 |
This looks fairly addressed by #38597 so closing. We can reopen a new issue if there were specific followups needed |
pd.Index
acceptsname
- greatpd.MultiIndex
acceptsnames
- after all, they are multiplename
spd.Index
acceptsnames
for compatibility withpd.MultiIndex
- well, OK, after all, that constructor can result in aMultiIndex
pd.Index
should accept ( BUG: Creating Index name usingnames
names argument, doesn't set index name #19082 )names
even when it results in a flat index - well, OK, still for compatibilitypd.MultiIndex
acceptsname
for compatibility - wait, wasn'tnames
already provided for compatibility?! OK, forget it, go forname
.pd.Index
acceptsname
even for multiple levels' names, for compatibility - withMultiIndex
, which accepts it for compatibility withpd.Index
- aaaaalrightpd.Index
subclasses (which will never result in multiple levels, and which currently in some cases discardnames
, in other cases raise an error) should acceptnames
for compatibility - no, wait.Proposal 1
There is one way to have compatibility across any kind of
Index
(sub)class constructor, and it isname
. When the constructor results in aMultiIndex
(which can happen withpd.Index
orpd.MultiIndex
), thenname
should be list-like, and each level will "receive" the respective componentin those cases - and only in those cases - in which a constructor actually results in a
MultiIndex
,names
can be used as an alias forname
. In all other cases, it is not accepted.or alternatively:
(b) (my preference)
names
is deprecated (in all cases in which it is currently supported, and remains unsupported in other constructors/cases)(c) (tradeoff between mental health and backward compatibility)
names
is supported inpd.MultiIndex
, still supported but deprecated inpd.Index
when resulting inMultiIndex
(and remains unsupported in other constructors/cases)Corollary:
names
will not be supported by any constructor that is notpd.Index
orpd.MultiIndex
.Notice that a 1-level
MultiIndex
is still aMultiIndex
. That is,pd.Index([('a',), ('b',)], name=('only_one_name',))
will still workpd.Index([('a',), ('b',)], names=('only_one_name',))
will still work (at least as long as we don't deprecatenames
entirely)pd.Index([('a',), ('b',)], name='only_one_name')
will still not workProposal 2
There is one way to have compatibility across any kind of
Index
(sub)class constructor, and it isnames
, which must always be list-like.In those cases in which the constructor results in a flat index,
name
is also accepted, and interpreted asnames=(name,)
; insteadname
is deprecated forpd.MultiIndex
, and forpd.Index
when resulting in aMultiIndex
(even if with 1 level)Corollary:
__new__
of non-MultiIndex
subclasses to convertnames
toname
The text was updated successfully, but these errors were encountered: