Skip to content
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: Change core paths to api.typing in URLs #55626

Merged
merged 1 commit into from
Oct 22, 2023

Conversation

rhshadrach
Copy link
Member

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

@phofl pointed out that #27522 would change the URLs in the API docs from pandas.core to pandas._core. We shouldn't be leaking the locations of the internals via the URLs. While pandas.api.typing is perhaps a little odd, it is at least public.

cc @jorisvandenbossche @jbrockmendel @mroeschke

Copy link
Member

@phofl phofl left a comment

Choose a reason for hiding this comment

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

Thx for addressing this

We might want to add something specifically for docs at some point

@mroeschke mroeschke added this to the 2.2 milestone Oct 22, 2023
@mroeschke mroeschke merged commit 5c7d89d into pandas-dev:main Oct 22, 2023
@mroeschke
Copy link
Member

Good catch. Thanks @rhshadrach

Copy link
Member

@jbrockmendel jbrockmendel left a comment

Choose a reason for hiding this comment

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

lgtm

@rhshadrach rhshadrach mentioned this pull request Dec 1, 2023
5 tasks
@jorisvandenbossche
Copy link
Member

I missed this notification at the time, but remembering it now because it came up in the discussion about deprecating core in the call yesterday.

Two comments:

  • This will actually break all existing URL out in the wild. Generally, when we do such a renaming, we add redirects for the existing pages (we have that mechanism set up, so it's just a matter of adding all impacted pages redirects.csv, AFAIK)
  • Given that changing urls is a bit annoying, we might want to give the final location a second thought. If we think about "we might want to add something specifically for docs at some point" to quote Patrick above (DOC: Provide a public place for users to link to our documentation #55632), we might better to that now, before those urls start to be used in the stable docs, to avoid breaking the urls a second time

I am all for breaking those urls once so we can remove the "core" out of those long references (which we need to deprecating core anyway, but those long references to "pandas.core.groupby.<..>" in the public docs have also always bothered me).
But personally I am not sure if pandas.api.typing is the ideal place to put those.

I will move the second item about what's the best location to use for the docs to the issue Patrick opened: #55632

@rhshadrach rhshadrach deleted the docs_public_api_paths branch December 14, 2023 20:12
This was referenced Dec 14, 2023
rhshadrach added a commit that referenced this pull request Dec 18, 2023
phofl pushed a commit that referenced this pull request Dec 19, 2023
cbpygit pushed a commit to cbpygit/pandas that referenced this pull request Jan 2, 2024
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.

5 participants