-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
DOC: Add missing attributes and methods to MultiIndex API documentation #50594
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
DOC: Add missing attributes and methods to MultiIndex API documentation #50594
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, not sure if there was a reason for not being there, I guess they were just forgotten.
CI failures are unrelated.
I see now that this only partially addresses the issue. I changes The problem seems to be in https://github.com/pandas-dev/pandas/blob/main/doc/source/reference/indexing.rst#multiindex I'm not fully sure how Sphinx manages those autosummaries. Seems like some of the attributes/methods are added automatically, while many others are added manually in different sections. This doesn't only affect Maybe I can suggest to move What do you think @Elvislim1991 ? Thanks for working on this! |
Ya, I realize the issues on the sidebar as well. As you said, It happens on all the methods so I thought it's good to keep it. I think it will be cleaner to just show the higher level as well. I can try doing that as well in this PR. What do you think @datapythonista? For the IndexSlice, I agree for aesthetics better to place it out of MultiIndex but close to it as IndexSlice is closely related to it. |
I expect this PR, including moving IndexSlice to be non-controversial and merged quickly. While grouping things in the sidebar can be trickier, and also more opinionated. If we use separate PRs, we'll get this part merged faster, and we won't need to keep reviewing the changes here while discussing about the sidebar grouping. No big deal, since changes here are small, but that's why I suggested splitting the PRs this way. But up yo you. |
Okay, I will just touch the IndexSlice for this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks for the work on this @Elvislim1991
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I realize now that IndexSlice
is under the MultiIndex
category, not with its own. Moving it to its own is what makes the CI fail.
It makes sense that it's under MultiIndex
, but at the same time it makes the navigation confusing, since it splits the MultiIndex
methods and attributes, which is not great. I'm not sure what's the best here. Maybe I'd revert this for now, and see in a separate PR if something can be done unifying MultiIndex
objects. And we see after that what makes sense to do with IndexSlice
.
as It's under multiIndex category
Ok, I will move back IndexSlice under MultiIndex. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm, thanks @Elvislim1991
Thanks @Elvislim1991 |
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.