Skip to content

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

Merged

Conversation

Elvislim1991
Copy link
Contributor

@Elvislim1991 Elvislim1991 commented Jan 5, 2023

Copy link
Member

@datapythonista datapythonista left a 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.

@datapythonista datapythonista changed the title [DOC] Add attributes and methods to MultiIndex documentation page to make i… DOC: Add missing attributes and methods to MultiIndex API documentation Jan 6, 2023
@datapythonista
Copy link
Member

I see now that this only partially addresses the issue. I changes closes in the description to xref for now, so we don't close the issue yet if this is merged. But would be great if you can also fix the problem in the left side menu, where some methos/attributes of MultiIndex are under the MultiIndex object, and some are listed independently after IndexSlice.

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 MultiIndex.

Maybe I can suggest to move IndexSlice to not be in the middle of MultiIndex in this PR, and then address the problem in the sidebar separately.

What do you think @Elvislim1991 ? Thanks for working on this!

@Elvislim1991
Copy link
Contributor Author

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.

@datapythonista
Copy link
Member

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.

@Elvislim1991
Copy link
Contributor Author

Okay, I will just touch the IndexSlice for this PR.

Copy link
Member

@datapythonista datapythonista left a 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

Copy link
Member

@datapythonista datapythonista left a 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.

@Elvislim1991
Copy link
Contributor Author

Ok, I will move back IndexSlice under MultiIndex.

Copy link
Member

@datapythonista datapythonista left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, thanks @Elvislim1991

@mroeschke mroeschke added this to the 2.0 milestone Jan 9, 2023
@mroeschke mroeschke merged commit 7c11b40 into pandas-dev:main Jan 9, 2023
@mroeschke
Copy link
Member

Thanks @Elvislim1991

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants