-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
DISC: add accessor attributes to Index for consistency with Series #17134
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
Comments
I'd support this. It makes writing generic downstream code easier. |
I would also support the idea, although I wouldn't implement it like that, because this would give you not the selection of options on tab completion (and also gives the possibility to misuse the accessor and use a wrong method eg |
@jorisvandenbossche That's a good point. Shouldn't be too difficult to do implement the "right" way. It'll take some small edits to |
I think this is ok. This makes Index/Series more consistent which is a good. thing. |
One thing to discuss is what it should return. In the PR you just opened, you say:
But it could also return the same as the plain
IMO it should return [74], not [75]. |
this should for sure be [74]. In fact the impl is pretty trivial (and indicated at the top of the PR). |
In one of the previous ones, but not in the latest: #17204 (I have to admit I am a bit lost in all the different PRs with related content) |
Sorry for the inundation. I'll be slowing down shortly. @jreback & @jorisvandenbossche The main advantage of the [75] version in #17204 is that it make |
No need to slow down! It is just that it can be a confusing when multiple PRs try to solve related / the same problems in different ways, and then you need to carefully explain there what the PR does, what the difference / relationship is with the other open PRs.
It makes the output for Series and Index indeed more similar, but, it makes the output for Index completely inconsistent with other index attributes. So I still think it should be like [74]. |
This needs to just use the simpler implementation (e.g. |
I'm on board with @jreback's suggestion. @jorisvandenbossche is your original concern about the |
Broken off of #17117 for discussion, xref #8162, #17061, recent mailing list thread.
With a datetime-like column we can access year, hour, ... with
self.dt.foo
. With a DatetimeIndex (PeriodIndex, ...) we access these attributes directlyself.foo
without the.dt
. I'd like to add a.dt
property to the appropriate Index subclasses so that these attributes can be accessed symmetrically. i.e. instead of:we can just use
year = obj.dt.year
regardless.The implementation is three lines in core.indexes.datetimelike.DatetimeIndexOpsMixin`:
Thoughts?
The text was updated successfully, but these errors were encountered: